Skip to content
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

fix two twice delivery on sync #560

Merged
merged 5 commits into from
Aug 27, 2023
Merged

fix two twice delivery on sync #560

merged 5 commits into from
Aug 27, 2023

Conversation

pfi79
Copy link
Contributor

@pfi79 pfi79 commented Jul 17, 2023

Proposal delivery can come from 2 goroutines: Controller and ViewChanger.
4 places:

Goroutines are fairly independent. It is required to ensure that delivery is not at the same time and that there is no twice delivery.
To prevent at the same time delivery, all deliveries are performed from under the syncLock mutex.
To prevent twice delivery, the Checkpoint parameter is implemented. Before delivery, the code reads Checkpoint and decides if the proposal should be delivered. After delivery, the code must set a new value in Checkpoint. Without this, double delivery is possible.

Found the last place where proposals can be delivered and Checkpoint is not set. This pull request fixes this error.

Signed-off-by: Фёдор Партанский <[email protected]>
@pfi79 pfi79 requested a review from C0rWin July 17, 2023 16:57
@C0rWin
Copy link
Contributor

C0rWin commented Jul 27, 2023

Can you please provide a unit-test which clearly demonstrates the issue you are addressing in the commit?

@pfi79 pfi79 closed this Aug 5, 2023
@pfi79 pfi79 reopened this Aug 5, 2023
@pfi79
Copy link
Contributor Author

pfi79 commented Aug 5, 2023

Can you please provide a unit-test which clearly demonstrates the issue you are addressing in the commit?

So far I have not been able to create a test in which this error always appears.
But I will add the test.log of the test in which this error occurred

Signed-off-by: Фёдор Партанский <[email protected]>
@pfi79
Copy link
Contributor Author

pfi79 commented Aug 14, 2023

Can you please provide a unit-test which clearly demonstrates the issue you are addressing in the commit?

I added a test.

Copy link
Contributor

@C0rWin C0rWin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@C0rWin C0rWin merged commit 154c866 into v2 Aug 27, 2023
3 checks passed
@C0rWin C0rWin deleted the two_twice_delivery_on_sync branch August 27, 2023 22:33
pfi79 added a commit that referenced this pull request Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants