This demonstrates a speedup in confirmation rate during initial boots… #3644
Conversation
|
This looks like the issue I found with announcements being dropped due to max_channel_requests being exceeded. Looking at that PR it removes max_channel_requests from the flow, probably could clean the code up a little by removing
And confirm_req_batches_max could be removed from config I think. Reference: #3574 (comment) |
clemahieu
force-pushed
the
confirmation_limiter
branch
from
e0b7f8f to
6dfa522
Compare
clemahieu
force-pushed
the
confirmation_limiter
branch
from
3ba40f1 to
53dbeca
Compare
clemahieu
force-pushed
the
confirmation_limiter
branch
from
53dbeca to
fd906fc
Compare
…nnel rather than by a fixed number.
clemahieu
force-pushed
the
confirmation_limiter
branch
from
fd906fc to
9e39e23
Compare
nano/node/confirmation_solicitor.cpp
Outdated
| @@ -70,7 +69,7 @@ bool nano::confirmation_solicitor::add (nano::election const & election_a) | |||
| if (!exists || !is_final || different) | |||
| { | |||
| auto & request_queue (requests[rep.channel]); | |||
| if (request_queue.size () < max_channel_requests) | |||
| if (!rep.channel->full ()) | |||
There was a problem hiding this comment.
Completely filling up the buffers is possibly not a good idea. It might starve other processes of network bandwidth. We are going from a value of 2 to thousands. How about half or quarter full?
|
Looks good to me, but @dsiganos suggestions make sense, it is worth to improve the function names he pointed out. |
channel::full() was calling channel::max() instead of socket::full(), which also exists. So it makes sense for consistency to rename it.
…trapping due to conservative limiters in the confirmation_solicitor. Rather than limiting with a constant number, it limits based on feedback directly from network channels.
Rather than limiting on a per-channel basis, a lot of complexity can be removed with by limiting the total number of active elections based on feedback from the confirmation rate.