-
Notifications
You must be signed in to change notification settings - Fork 790
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
Conversation
4aa3cf4
to
101790f
Compare
101790f
to
44eb324
Compare
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 ? |
6ceacc1
to
44eb324
Compare
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. |
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:
This PR addresses this problem by:
node.bootstrap_block (...)
. This in theory should be sufficient, but I wanted to eliminate other inefficiencies that made this bug possible.