Conversation
Test Results for Commit 6924d54Pull Request 4806: Results Test Case Results
Last updated: 2024-12-10 13:42:21 UTC |
| // Do not throttle accounts that are probably fully synced | ||
| if (sent && fails == 0) | ||
| { | ||
| nano::lock_guard<nano::mutex> lock{ mutex }; | ||
| accounts.timestamp_set (account); | ||
| } |
There was a problem hiding this comment.
By setting the timestamp to current value ( accounts.timestamp_set (account); ) a cooldown period in next_priority () is applied to this account. Is that the desired behavior ?
There was a problem hiding this comment.
This should work as intended. It might be counterintuitive because we want to throttle only accounts with potentially more blocks; it's to avoid requesting blocks from the same frontier over and over before the current batch is processed by block processor.
The flow is something like:
if (likely to have more blocks)
{
set cooldown timestamp
}
// ... later in the block processing logic
if (last received block processed)
{
reset cooldown timestamp
}
There was a problem hiding this comment.
Okay, that makes sense.
I got confused by the comment
// Do not throttle accounts that are probably fully synced
So if I understand correctly,the goal is to rapidly deprioritise fully synced accounts by requesting again without throttle.
There was a problem hiding this comment.
Yes, I updated the comment to make the intention clear
| @@ -204,8 +206,24 @@ bool nano::bootstrap_service::send (std::shared_ptr<nano::transport::channel> co | |||
|
|
|||
| // TODO: There is no feedback mechanism if bandwidth limiter starts dropping our requests | |||
There was a problem hiding this comment.
Is this now handled by the added callback ? Or is the TODO about something else ?
There was a problem hiding this comment.
This todo is now out of date
fefadf2 to
6924d54
Compare
This fixes a few more problems with bootstrap that I noticed when working on traffic shaping.