do not update pointer for failed audits

Change-Id: If88dce8928db28d6f53c3dc771e14ea97aae9661
This commit is contained in:
Moby von Briesen 2019-12-16 10:42:26 -05:00
parent 45f8bb5084
commit ab777e823e
3 changed files with 2 additions and 74 deletions

View File

@ -226,7 +226,6 @@ func TestReverifyFailMissingShareNotVerified(t *testing.T) {
shareSize := pointer.GetRemote().GetRedundancy().GetErasureShareSize()
pieces := pointer.GetRemote().GetRemotePieces()
origNumPieces := len(pieces)
rootPieceID := pointer.GetRemote().RootPieceId
limit, privateKey, err := orders.CreateAuditOrderLimit(ctx, bucketID, pieces[0].NodeId, pieces[0].PieceNum, rootPieceID, shareSize)
require.NoError(t, err)
@ -269,14 +268,6 @@ func TestReverifyFailMissingShareNotVerified(t *testing.T) {
require.Len(t, report.PendingAudits, 0)
// expect no failed audit
require.Len(t, report.Fails, 0)
// expect that bad node is no longer in the pointer
pointer, err = satellite.Metainfo.Service.Get(ctx, path)
require.NoError(t, err)
assert.Len(t, pointer.GetRemote().GetRemotePieces(), origNumPieces-1)
for _, p := range pointer.GetRemote().GetRemotePieces() {
assert.NotEqual(t, p.NodeId, pieces[0].NodeId)
}
})
}

View File

@ -243,11 +243,6 @@ func (verifier *Verifier) Verify(ctx context.Context, path storj.Path, skip map[
for _, pieceNum := range pieceNums {
failedNodes = append(failedNodes, shares[pieceNum].NodeID)
}
// remove failed audit pieces from the pointer so as to only penalize once for failed audits
err = verifier.removeFailedPieces(ctx, path, pointer, failedNodes)
if err != nil {
verifier.log.Warn("Verify: failed to delete failed pieces", zap.Binary("Segment", []byte(path)), zap.Error(err))
}
successNodes := getSuccessNodes(ctx, shares, failedNodes, offlineNodes, unknownNodes, containedNodes)
@ -551,17 +546,12 @@ func (verifier *Verifier) Reverify(ctx context.Context, path storj.Path) (report
}
if errs2.IsRPC(err, rpcstatus.NotFound) {
// Get the original segment pointer in the metainfo
oldPtr, err := verifier.checkIfSegmentAltered(ctx, pending.Path, pendingPointer)
_, err := verifier.checkIfSegmentAltered(ctx, pending.Path, pendingPointer)
if err != nil {
ch <- result{nodeID: pending.NodeID, status: success}
verifier.log.Debug("Reverify: audit source deleted before reverification", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID), zap.Error(err))
return
}
// remove failed audit pieces from the pointer so as to only penalize once for failed audits
err = verifier.removeFailedPieces(ctx, pending.Path, oldPtr, storj.NodeIDList{pending.NodeID})
if err != nil {
verifier.log.Warn("Reverify: failed to delete failed pieces", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID), zap.Error(err))
}
// missing share
ch <- result{nodeID: pending.NodeID, status: failed}
verifier.log.Debug("Reverify: piece not found (audit failed)", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID), zap.Error(err))
@ -583,17 +573,12 @@ func (verifier *Verifier) Reverify(ctx context.Context, path storj.Path) (report
ch <- result{nodeID: pending.NodeID, status: success}
verifier.log.Debug("Reverify: hashes match (audit success)", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID))
} else {
oldPtr, err := verifier.checkIfSegmentAltered(ctx, pending.Path, pendingPointer)
_, err := verifier.checkIfSegmentAltered(ctx, pending.Path, pendingPointer)
if err != nil {
ch <- result{nodeID: pending.NodeID, status: success}
verifier.log.Debug("Reverify: audit source deleted before reverification", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID), zap.Error(err))
return
}
// remove failed audit pieces from the pointer so as to only penalize once for failed audits
err = verifier.removeFailedPieces(ctx, pending.Path, oldPtr, storj.NodeIDList{pending.NodeID})
if err != nil {
verifier.log.Warn("Reverify: failed to delete failed pieces", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID), zap.Error(err))
}
verifier.log.Debug("Reverify: hashes mismatch (audit failed)", zap.Binary("Segment", []byte(pending.Path)), zap.Stringer("Node ID", pending.NodeID),
zap.Binary("expected hash", pending.ExpectedShareHash), zap.Binary("downloaded hash", downloadedHash))
ch <- result{nodeID: pending.NodeID, status: failed}
@ -692,29 +677,6 @@ func (verifier *Verifier) GetShare(ctx context.Context, limit *pb.AddressedOrder
}, nil
}
// removeFailedPieces removes lost pieces from a pointer
func (verifier *Verifier) removeFailedPieces(ctx context.Context, path string, pointer *pb.Pointer, failedNodes storj.NodeIDList) (err error) {
defer mon.Task()(&ctx)(&err)
if len(failedNodes) == 0 {
return nil
}
var toRemove []*pb.RemotePiece
OUTER:
for _, piece := range pointer.GetRemote().GetRemotePieces() {
for _, failedNode := range failedNodes {
if piece.NodeId == failedNode {
toRemove = append(toRemove, piece)
continue OUTER
}
}
}
// Update the segment pointer in the metainfo
_, err = verifier.metainfo.UpdatePieces(ctx, path, pointer, nil, toRemove)
return err
}
// checkIfSegmentAltered checks if path's pointer has been altered since path was selected.
func (verifier *Verifier) checkIfSegmentAltered(ctx context.Context, segmentPath string, oldPointer *pb.Pointer) (newPointer *pb.Pointer, err error) {
defer mon.Task()(&ctx)(&err)

View File

@ -565,14 +565,6 @@ func TestVerifierMissingPieceHashesNotVerified(t *testing.T) {
assert.Len(t, report.Fails, 0)
assert.Len(t, report.Offlines, 0)
assert.Len(t, report.PendingAudits, 0)
// expect that bad node is no longer in the pointer
pointer, err = satellite.Metainfo.Service.Get(ctx, path)
require.NoError(t, err)
assert.Len(t, pointer.GetRemote().GetRemotePieces(), origNumPieces-1)
for _, p := range pointer.GetRemote().GetRemotePieces() {
assert.NotEqual(t, p.NodeId, piece.NodeId)
}
})
}
@ -733,23 +725,6 @@ func TestVerifierModifiedSegmentFailsOnce(t *testing.T) {
assert.Equal(t, report.Fails[0], piece.NodeId)
assert.Len(t, report.Offlines, 0)
require.Len(t, report.PendingAudits, 0)
// refetch the pointer
pointerAgain, err := satellite.Metainfo.Service.Get(ctx, path)
require.NoError(t, err)
report, err = audits.Verifier.Verify(ctx, path, nil)
require.NoError(t, err)
//verify no failures because that segment is gone
assert.Len(t, report.Successes, origNumPieces-1)
assert.Len(t, report.Fails, 0)
assert.Len(t, report.Offlines, 0)
require.Len(t, report.PendingAudits, 0)
for _, newPiece := range pointerAgain.GetRemote().GetRemotePieces() {
assert.NotEqual(t, newPiece.NodeId, piece.NodeId)
}
})
}