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

Fixes for hinted scheduler & vote cache #4334

Merged
merged 8 commits into from
Nov 8, 2023

Conversation

pwojcikdev
Copy link
Contributor

There was a bug in hinted scheduler, when if node hardware was sufficiently slow background tasks related to bootstrapping missing blocks would queue up excessively:

Screenshot 2023-11-07 at 02 06 04

This PR addresses this problem by:

  • Removing the problematic call to node.bootstrap_block (...). This in theory should be sufficient, but I wanted to eliminate other inefficiencies that made this bug possible.
  • Cleanup vote cache periodically. The default punning age is 5 minutes - votes received earlier than this threshold will be removed.
  • Adjust block check cooldown to avoid checking the same block too often.

clemahieu
clemahieu previously approved these changes Nov 7, 2023
@gr0vity-dev
Copy link
Contributor

gr0vity-dev commented Nov 8, 2023

I noticed that you first made the vacancy threshold configurable and in a later commit you removed the notify and the option altogether. By removing the notification, you limit the execution of the hinted scheduler to once per {check_interval} which is 1 second currently. Are there specific performance concerns with the previous approach ?

The duration of the vote cache cleanup is the same as the election duration (5 minutes). This means that if an election is not confirmed in the first round, all nodes need to vote on it again. Would it make sense to allow 2 election rounds before doing the cleanup ? Or was this done on purpose ?
I personally think that 5 minutes for the vote cache cleanup is fine but that the election duration should be reduced.

@pwojcikdev
Copy link
Contributor Author

There is a problem with this notification, where it will keep hinted scheduler very busy even though there is nothing to hint. This happens during bootstrapping when there is some network activity: vote cache gets filled, but the blocks aren't there - this then causes unnecessary churn. I removed that notification to avoid that, but it does have a negative effect of limiting hinted CPS to (Hinted AEC allocation = 500) / (Hinted loop interval = 1s). However, I think I have a better (and still simple) way to avoid this, I'll revert that last commit for now and try to address this issue later.

Vote cache will keep a vote for 5 minutes after the last seen vote, so in practice as long as any node actively generates votes (which is going to happen for 5 minutes during which election is active) the cache entry will be kept alive. That means it will be alive for 5 + 5 minutes, which is 2 election rounds.

@pwojcikdev pwojcikdev merged commit 58a8a6b into nanocurrency:develop Nov 8, 2023
29 of 34 checks passed
@qwahzi qwahzi added this to the V26.0 milestone Nov 20, 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.

4 participants