* init decoupling-post-worker code #197
Conversation
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
91ef2da
to
143b999
Compare
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
143b999
to
2d03626
Compare
|
@jennijuju The functions of removing duplicate sectors also needs to be merged （ types.go file ） . This modification can improve efficiency |
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
3 times, most recently
from
413873a
to
d670ec9
Compare
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
d670ec9
to
a72c001
Compare
|
@jennijuju The code has been debugged and verified, please review the merge |
types.go
Outdated
| @@ -131,3 +132,50 @@ type PrivateSectorInfo struct { | |||
| type AllocationManager interface { | |||
| Free() | |||
| } | |||
|
|
|||
| func DeDuplicatePrivateSectorInfo(ctx context.Context, sortPrivSectors *SortedPrivateSectorInfo) (SortedPrivateSectorInfo, error) { | |||
There was a problem hiding this comment.
Looking at code in Lotus it looks like it should be possible to execute this and split operations before creating the SortedPrivateSectorInfo.
I strongly suggest changing it to that.
The SortedPrivateSectorInfo type should only be used for directly passing sector information into FFI not for carrying sector information around or operating on them.
|
@Kubuxu Agree with you. We have a try |
|
@jennijuju @Kubuxu According to the review comments, the deduplication of SortedPrivateSectorInfo is performed in lotus, because the key member variables of SortedPrivateSectorInfo are private types and cannot be accessed at the lotus layer. We tried to perform deduplication before calling ffi.NewSortedPrivateSectorInfo and tested it, but proof verification Fail. Any suggestions for modification? |
types.go
Outdated
| } | ||
| if !flag { | ||
| break | ||
| } |
There was a problem hiding this comment.
The above code seems to sort sector infos by SectorNumber, but the original code sorts them by SealedCID, why the change?
Also if you want to sort, please use sort.Slice. See types.go#93
There was a problem hiding this comment.
@Kubuxu According to the logic before splitting,Split calculation results need to be sorted by SectorNumber
There was a problem hiding this comment.
@Kubuxu Any suggestions for modification?
There was a problem hiding this comment.
Because the members of the SortedProvateSectorInfo structure are private and cannot be operated in lotus, we think the DeDuplicatePrivateSectorInfo interface is necessary, or are there any other implementation suggestions? @Kubuxu
There was a problem hiding this comment.
Won't passing sectors deduplicated here to NewSortedPrivateSectorInfo sort them correctly? If not, can we change NewSortedPrivateSectorInfo to sort correctly?
There was a problem hiding this comment.
@magik6k We can change NewSortedPrivateSectorInfo to sort correctly, If It without affecting the logic of other functions
There was a problem hiding this comment.
@magik6k Modification has been submitted, please review
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
cc44596
to
5646546
Compare
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
e398ed6
to
9743801
Compare
lovel8
force-pushed
the
feat/decoupling-post-worker
branch
from
9743801
to
ca0c6ba
Compare
|
@magik6k The code has been debugged and verified, please review and merge |
There was a problem hiding this comment.
I still don't really understand the change in sorting - why sorting by SealedCID(CommR) worked before, and why we're changing it to sort by SectorNumber. Reading the rust code, the only thing I see is that the code doesn't really seem to care about sorting, at least in case of the monolith post methods.
Will merge this, if it's somehow broken it should be really easy to see that and fix
Merge based on：
filecoin-project/lotus#6943