Closed mennopruijssers closed 7 years ago
LGTM
Should we put a section about self-eviction to the architecture page in public docs?
I miss a parallelism factor to control how many concurrent pings we will be sending during self eviction. Is that still sth we want to have in this feature or are willing to take the hit on shutdown speed if you are pinging nodes that just went away? Same about my comment to use gossip.tick
, are we ok with the slower shutdown phase by including pingreqs in the process?
@thanodnl I wanted to start simple without parallelism and test out the impact on shutdown speed. If shutdown is taking too long we can improve by pinging in parallel. This would however, require some changes in the way the tick
function is currently implemented: currently it allows only one ping
running at a time.
@Motiejus probably yes but lets first try to stabilize the feature :)
I think the easiest is to not rely on the tick feature but extract the part that sends the ping (which is already quite abstract with sendPing
) into a reusable component and use that one instead. That would solve the pingReq
concerns at the same time. But we can start with this and indeed measure how fast a shutdown is before optimizing.
I think the easiest is to not rely on the tick feature but extract the part that sends the ping (which is already quite abstract with
sendPing
) into a reusable component and use that one instead. That would solve thepingReq
concerns at the same time. But we can start with this and indeed measure how fast a shutdown is before optimizing.
I changed to code to use sendPing
. Makes sense, thanks!
👍
LGTM
LGTM
Ping other members on self-eviction.
This PR is dependent on #299