Add throttling functionality to the ascending bootstrapper #4205
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… successful retrieval of blocks from other nodes. If there are no recent block retrievals, the bootstrapper will throttle itself and delay between requests. Bootstrapper will make one full pass of the ledger before throttling for quicker syncing on startup.
Disabled node.aggressive_flooding test
clemahieu
force-pushed
the
bootstrap_ascending_throttling
branch
from
563e40b
to
a1a0a9f
Compare
thsfs
reviewed
Apr 4, 2023
thsfs
reviewed
Apr 4, 2023
thsfs
reviewed
Apr 4, 2023
pwojcikdev
reviewed
Apr 4, 2023
| @@ -678,12 +717,23 @@ bool nano::bootstrap_ascending::run_one () | |||
| return success; | |||
| } | |||
|
|
|||
| void nano::bootstrap_ascending::throttle_if_needed () | |||
| { | |||
| if (!iterator.warmup () && throttle.throttled ()) | |||
There was a problem hiding this comment.
Doesn't 'throttle' object access need a lock?
There was a problem hiding this comment.
As it's written it assumes synchronisation is handled externally.
This seemed appropriate since it's a 1-to-1 association between bootstrap_ascending instance and throttle instance.
There was a problem hiding this comment.
Why then are you locking accesses of this class in other places, I believe your explanation is not correct and it is in fact being used from multiple threads.
There was a problem hiding this comment.
In other words, just move the lock acquired on line 725 above this if statement.
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the ability for the ascending bootstrapper to slow itself down when no progress is being made. This inability to slow down had a side effect of impacting our unit testing and required us to turn the request rate very low compared to synthetic and beta tests.
With the exception of the aggressive_flooding test, unit tests will now pass when the rate is up to 1024 from 128 having tested with 4096 previously.
This disables the aggressive_flooding test, which will be moved to load tests, since its resource requirement is high.