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.

Sign up for GitHub

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

Jump to bottom

Election flush #3265

Merged
merged 5 commits into from
May 6, 2021
Merged

Election flush #3265

merged 5 commits into from
May 6, 2021

Conversation

clemahieu
Copy link
Contributor

This corrects the implementation of flushing the election scheduler. Flushing will terminate when either:

  1. The election scheduler is stopped
  2. there are no more elections to schedule
  3. the active transaction container has no vacancy.

@clemahieu
Rewriting election_scheduler::flush so it will terminate if there is …
f088c61 
…no vacancy left in active_transactions or if it is stopped.

Renaming election_scheduler::observe to ::notify as that's what it's doing.
@clemahieu
Merge branch 'develop' into election_flush
5ffbaba
@clemahieu clemahieu added the bug label May 6, 2021
@clemahieu clemahieu added this to the V22.0 milestone May 6, 2021
@clemahieu clemahieu requested a review from dsiganos May 6, 2021 11:05
@thsfs thsfs self-requested a review May 6, 2021 13:03
thsfs
thsfs reviewed May 6, 2021
View reviewed changes
Copy link
Contributor

@thsfs thsfs left a comment

Choose a reason for hiding this comment

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

I'm not able to evaluate the whole functionality, but the changes seem ok. My only question is regarding the assertion check for the flush_vacancy unit test.

nano/core_test/election_scheduler.cpp Show resolved Hide resolved
theohax
theohax previously approved these changes May 6, 2021
View reviewed changes
@thsfs thsfs self-requested a review May 6, 2021 16:47
@clemahieu clemahieu merged commit b43c218 into develop May 6, 2021
@zhyatt zhyatt deleted the election_flush branch September 21, 2021 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thsfs thsfs thsfs approved these changes

@theohax theohax theohax left review comments

@dsiganos dsiganos Awaiting requested review from dsiganos

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
V22.0
Development

Successfully merging this pull request may close these issues.
3 participants
@clemahieu @theohax @thsfs