Mainlining of Fast Snap logic #804
Conversation
* fix(retrieval): handle pathed IPFS retrieval style, add basic retrieval tests (#720) * ipni: Announce addresses with port * update lotus * make gen --------- Co-authored-by: Rod Vagg <[email protected]>
magik6k
left a comment
magik6k
left a comment
There was a problem hiding this comment.
Some comments to my code, some comments around the merge. Overall looks really good, let's keep this moving
|
|
||
| for _, sectorRef := range sectorRefs { | ||
| if err := t.sc.Sectors.sindex.StorageLock(lkctx, sectorRef.ID(), storiface.FTNone, requestedTypes); err != nil { | ||
| ok, err := t.sc.Sectors.sindex.StorageTryLock(lkctx, sectorRef.ID(), storiface.FTNone, requestedTypes) |
There was a problem hiding this comment.
I'd honestly just entirely get rid of the sector storage lock stuff, it's basically entirely redundant with harmonytask managing pipelines
There was a problem hiding this comment.
Is this a blocker? Otherwise, this PR is too big already and it should become an (easy) GitHub issue.
There was a problem hiding this comment.
Is this a blocker? Otherwise, this PR is too big already and it should become an (easy) GitHub issue.
There was a problem hiding this comment.
iiuc those changes were re-implemented in a slightly different way in main, so we don't really want to backport anything ipni-related.
Initially, this is only a merge, but I need it reviewed as a number of things were changed by both sides. I generally went with the "everyone's right" approach, but where functions were restructured by both sides, this may be incorrect.
DO NOT SUBMIT
Once approved, it still needs other changes to avoid breaking anyone. This PR will be a start for those changes.