-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixed bug for sector FinalizeFailed when set FinalizeEarly=true #6649
Conversation
@@ -123,7 +123,7 @@ var fsmPlanners = map[SectorState]func(events []statemachine.Event, state *Secto | |||
on(SectorRetrySubmitCommit{}, SubmitCommit), | |||
), | |||
CommitAggregateWait: planOne( | |||
on(SectorProving{}, FinalizeSector), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be ok, calling finalize for the second time should just see that the sector is already finalized, and go to Proving that way
What errors are you seeing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before SubmitCommitAggregate and everything is find and the sectors were finalized, also the data were downloaded to the final storage server, After SubmitCommitAggregate the sectors state keep firing the following steps:
SubmitCommitAggregate(on.SectorCommitAggregateSent)->CommitWait(on.SectorProving)->FinalizeSector
since the sectors were deleted before the SubmitCommitAggregate finalize operation and the Finalize call now will fire SectorFinalizeFailed CUZ of sector not found error, it then fire FinalizeFailed and
FinalizeFailed: planOne(
on(SectorRetryFinalize{}, FinalizeSector),
),
``
it keeps firing FinalizeSector and it keeps retry finalize and the sectors state will always be FinalizedFailed.
that's what happened during our local tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this PR will cause a new issue for when if the FinalizeEarly were set to false and it will not call the finalize after SectorProving
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be ok, calling finalize for the second time should just see that the sector is already finalized, and go to Proving that way
What errors are you seeing?
The second FinalizeSector will cause the FinalizeFailed. Now it fixed in PR #6653 , Thanks.
#6653 should fix the underlying Finalize issue, it would be really great if you were able to see if it fixes the issue for you |
Yep, i see. and your solution is indeed better, i had tested it in local dev-net and it running well, thanks. |
No description provided.