Skip to content

Commit

Permalink
satellite/repair: don't remove expired segments from repair queue
Browse files Browse the repository at this point in the history
It's impossible to time correctly this check. The segment may expire
just at the time we upload the repaired pieces to new storage nodes.
They will reject this as expired and the repair will fail.

Also, we penalize storage nodes with audit failure only if they fail
piece hash verification, i.e. return incorrect data, but only if they
have already deleted the piece.

So, it would be best if the repair service does not care about object
expiration at all. This is a responsibility of another service.

Removing this check will also simplify how we migrate this code
correctly to the metabase.

Change-Id: I09f7b372ae2602daee919a8a73cd0475fb263cd2
  • Loading branch information
kaloyan-raev committed Feb 2, 2021
1 parent 22bc69a commit 339d121
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 9 deletions.
1 change: 0 additions & 1 deletion monkit.lock
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ storj.io/storj/satellite/repair/repairer."download_failed_not_enough_pieces_repa
storj.io/storj/satellite/repair/repairer."healthy_ratio_after_repair" FloatVal
storj.io/storj/satellite/repair/repairer."healthy_ratio_before_repair" FloatVal
storj.io/storj/satellite/repair/repairer."repair_attempts" Meter
storj.io/storj/satellite/repair/repairer."repair_expired" Meter
storj.io/storj/satellite/repair/repairer."repair_failed" Meter
storj.io/storj/satellite/repair/repairer."repair_nodes_unavailable" Meter
storj.io/storj/satellite/repair/repairer."repair_partial" Meter
Expand Down
6 changes: 3 additions & 3 deletions satellite/repair/repair_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ func testCorruptDataRepairSucceed(t *testing.T, inMemoryRepair bool) {
// - Call checker to add segment to the repair queue
// - Modify segment to be expired
// - Run the repairer
// - Verify segment is no longer in the repair queue.
func TestRemoveExpiredSegmentFromQueue(t *testing.T) {
// - Verify segment is still in the repair queue. We don't want the data repairer to have any special treatment for expired segment.
func TestRepairExpiredSegment(t *testing.T) {
testplanet.Run(t, testplanet.Config{
SatelliteCount: 1,
StorageNodeCount: 10,
Expand Down Expand Up @@ -482,7 +482,7 @@ func TestRemoveExpiredSegmentFromQueue(t *testing.T) {
// Verify that the segment was removed
count, err = satellite.DB.RepairQueue().Count(ctx)
require.NoError(t, err)
require.Equal(t, count, 0)
require.Equal(t, count, 1)
})
}

Expand Down
5 changes: 0 additions & 5 deletions satellite/repair/repairer/segments.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,6 @@ func (repairer *SegmentRepairer) Repair(ctx context.Context, path storj.Path) (s
return true, invalidRepairError.New("cannot repair inline segment")
}

if !pointer.ExpirationDate.IsZero() && pointer.ExpirationDate.Before(time.Now().UTC()) {
mon.Meter("repair_expired").Mark(1) //mon:locked
return true, nil
}

mon.Meter("repair_attempts").Mark(1) //mon:locked
mon.IntVal("repair_segment_size").Observe(pointer.GetSegmentSize()) //mon:locked

Expand Down

0 comments on commit 339d121

Please sign in to comment.