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 vote_generator loop inconsistency during high load #2095

Merged
merged 5 commits into from
Jun 27, 2019

Conversation

guilhermelawless
Copy link
Contributor

@guilhermelawless guilhermelawless commented Jun 17, 2019

Adds a vote_generator_threshold config option ( between 1 and 12 ) and new logic for the vote generator loop. The goal is to make it behave like a deadline timer which is reset when new hashes arrive, and when fired sends the hashes.

  • The loop still waits on a condition_variable for up to vote_generator_delay (50ms default)
  • The cv can be notified by vote_generator::add when a new hash arrives, which makes the loop not send the votes yet, but wait for more. This happens when hashes are arriving fast.
  • When the cv is notified, that means hashes are not arriving sufficiently fast, so the loop sends the votes

The addition of vote_generator_threshold helps in configuring what the "high load threshold" should be.

Approximate equation (assumes constant rate during sleep time): hash_arrival_rate * sleep_time = hash . By setting hashes to our threshold (default 3), and sleeping the default 50ms, we expect to enter the high load mode at a vote rate of 60. Above that number, if consistent, and we should only see packs of 12 votes. In reality it's lower, at 40 or 45.

So we can play with the delay and threshold to achieve a desired configuration. By default, anything below 40 tps and the node should not take more than 50ms to vote for a transaction. Above Around 40-60 it should take around 200ms, decreasing for higher hash arrival rates.


I've tested this pretty successfully on beta already. Would be interesting to see in RC5 if no issues arise. With a 100tps push of ~2000 blocks by Json:
117,63,0,0,0,0,1,0,0,0,0,176 , where the first number is votes with 1 hash and the last is votes with 12 hashes

@clemahieu
Copy link
Contributor

What's the reason for it taking 200ms at 40-60 tps?

@guilhermelawless
Copy link
Contributor Author

guilhermelawless commented Jun 25, 2019

@clemahieu
First, 3 hashes arrive in 50ms (if less than 3 after 50ms we just pack and send).
Afterwards:

  • the worst case for latency is one hash every 50ms. This would mean 50+50(12-3) = 500ms.
  • the average case will see a constant hash arrival rate during the hash gathering period (3h/50ms), resulting in 12 votes after 200ms.

I should have said initially that if the rate is any higher than 3h/50ms then it takes less than 200ms, probably this is why you asked. There is no lower bound.

@zhyatt zhyatt requested review from clemahieu and removed request for wezrule June 26, 2019 17:56
Copy link
Contributor

@cryptocode cryptocode left a comment

Choose a reason for hiding this comment

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

Looks good in my tests

@zhyatt zhyatt added the performance Performance/resource utilization improvement label Jun 26, 2019
@zhyatt zhyatt added this to the V19.0 milestone Jun 26, 2019
@zhyatt zhyatt merged commit d404dcf into nanocurrency:master Jun 27, 2019
@guilhermelawless guilhermelawless deleted the vote-generator-deadline branch June 27, 2019 05:33
argakiig pushed a commit that referenced this pull request Jun 27, 2019
* Fix vote_generator loop inconsistency during high load.

* Increase deadline for bulk_pull_account.basics test

* Revert "Increase deadline for bulk_pull_account.basics test"

This reverts commit 4404c4c.

* Simplify config option

* Use a flag as the wait predicate to prevent getting stuck with hashes between threshold and 11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance/resource utilization improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants