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 confirmation_height.election_winner_details_clearing #3741

Merged

Conversation

dsiganos
Copy link
Contributor

There is a race condition between a block getting confirmed and setting
of statistics counters. In this case, the counter is incremented in an
observer callback that is executed after the block is confirmed.

One could say that the problem is that the stat counters should updated
before the block is confirmed but I think that is unrealistic to achieve
and possibly not even the right approach.

Converting the check to an ASSERT_TIMELY fixes the problem.
It seems likely that other test cases will have this problem too.

There is a race condition between a block getting confirmed and setting
of statistics counters. In this case, the counter is incremented in an
observer callback that is executed after the block is confirmed.

One could say that the problem is that the stat counters should updated
before the block is confirmed but I think that is unrealistic to achieve
and possibly not even the right approach.

Converting the check to an ASSERT_TIMELY fixes the problem.
It seems likely that other test cases will have this problem too.
@dsiganos dsiganos added the unit test Related to a new, changed or fixed unit test label Feb 15, 2022
@theohax
Copy link
Contributor

theohax commented Feb 15, 2022

quorum_minimum_update_weight_before_quorum_checks failed in the CI but it looks like a scheduler flush thing, I'll have a look at it -- I see it's not yet on the list

@dsiganos dsiganos merged commit e51b3d6 into nanocurrency:develop Feb 15, 2022
@dsiganos dsiganos deleted the fix_election_winner_details_clearing branch February 15, 2022 15:56
@zhyatt zhyatt added this to the V24.0 milestone Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unit test Related to a new, changed or fixed unit test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants