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

Hinted election scheduler #3944

Merged

Conversation

pwojcikdev
Copy link
Contributor

This PR adds an additional election scheduler that prioritises elections by cached tally weight, so elections with the most voting weight already cached (thus under normal conditions more likely to be quickly confirmed) are started first. During testing this demonstrated a significant speedup when backlog is present and nodes are desynchronised.

clemahieu
clemahieu previously approved these changes Sep 8, 2022
* However, it is possible that AEC will be temporarily overfilled in case it's running at full capacity and election hinting or manual queue kicks in.
* That case will lead to unwanted churning of elections, so this allows for AEC to be overfilled to 125% until erasing of elections happens.
*/
return node.active.vacancy () < -(node.active.limit () / 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of overfilling though existing behaviour already did this so I don't see a reason to change it here. In the future I think election sets with different limits should be different containers.

* Creates `count` 1 raw sends from genesis to unique accounts and corresponding open blocks.
* The genesis chain is then confirmed, but leaves open blocks unconfirmed.
*/
std::vector<std::shared_ptr<nano::block>> setup_independent_blocks (nano::test::system & system, nano::node & node, int count)
Copy link
Contributor

Choose a reason for hiding this comment

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

These would probably be useful in the ::test namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will move those in separate PR, a new file with all the helpers needs to be created and I don't want to pollute this PR now

@dsiganos
Copy link
Contributor

dsiganos commented Sep 8, 2022

I think we should be passing a logger into these classes and have the ability to log messages.

@dsiganos
Copy link
Contributor

dsiganos commented Sep 8, 2022

I think the hinted scheduler should have some counters counting events.
So we can monitor its efficiency and for troubleshooting.

dsiganos
dsiganos previously approved these changes Sep 8, 2022
@pwojcikdev pwojcikdev dismissed stale reviews from dsiganos and clemahieu via b5aaa0a September 9, 2022 10:15
@pwojcikdev
Copy link
Contributor Author

@dsiganos I added stat logging, should cover all the interesting pathways

@pwojcikdev pwojcikdev merged commit 2dcf1ad into nanocurrency:develop Sep 13, 2022
@pwojcikdev pwojcikdev deleted the prs/hinted-election-scheduler branch September 13, 2022 13:11
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.

None yet

3 participants