This demonstrates a speedup in confirmation rate during initial boots…#3644
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) |
e0b7f8f to
6dfa522
Compare
3ba40f1 to
53dbeca
Compare
53dbeca to
fd906fc
Compare
…nnel rather than by a fixed number.
fd906fc to
9e39e23
Compare
nano/node/confirmation_solicitor.cpp
Outdated
| { | ||
| 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.