storage: Fix FinalizeSector with sectors in stoage paths #6653
Conversation
| @@ -14,7 +14,7 @@ func TestDealsWithSealingAndRPC(t *testing.T) { | |||
|
|
|||
| kit.QuietMiningLogs() | |||
|
|
|||
| var blockTime = 1 * time.Second | |||
| var blockTime = 50 * time.Millisecond | |||
There was a problem hiding this comment.
Double checking: this is just to speed up the test and has no behavior change?
| selector := newExistingSelector(m.index, sector.ID, storiface.FTCache|storiface.FTSealed, false) | ||
|
|
||
| err := m.sched.Schedule(ctx, sector, sealtasks.TTFinalize, selector, | ||
| m.schedFetch(sector, storiface.FTCache|storiface.FTSealed|unsealed, storiface.PathSealing, storiface.AcquireMove), | ||
| m.schedFetch(sector, storiface.FTCache|storiface.FTSealed|unsealed, pathType, storiface.AcquireMove), |
There was a problem hiding this comment.
I am brand new to this code so I have some very basic questions.
If we have already finalized this sector id then all of the sealedStores above will have store.CanSeal equal to false ?
Why do we need the worker to finalize again if the pathType is PathStorage? Do sectors in long term storage need to be finalized again during subsequent finalize calls? Similarly why do we need to do the MoveStorage below in this case? Aren't all the sectors already in long term storage?
There was a problem hiding this comment.
If we have already finalized this sector id then all of the sealedStores above will have store.CanSeal equal to false ?
In some setups storage paths can be used both for sealing and storage
Why do we need the worker to finalize again if the pathType is PathStorage?
It's possible that the sector wasn't properly finalized for a number of other reasons (sealing path switched to storage, sectors moved manually, etc.). The finalize operation is quite cheap (removing/trimming some files), so there is little downside to calling it twice for the added robustness.
Similarly why do we need to do the MoveStorage below in this case? Aren't all the sectors already in long term storage?
MoveStorage already has that check internally
ff9c7a0 to
8a94ab6
Compare
This fixes the case where sectors could be moved in one call to Finalize to a long-term storage, and then fail in second call to Finalize because we expected to find those files still in sealing storage
Related to #6651 and #6649