-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP - Reduce the Load Induced by the Polling Service #211
Conversation
Signed-off-by: Peter Neuroth <[email protected]>
We use the queue to determine which peer is the next to be sent the poll message. Signed-off-by: Peter Neuroth <[email protected]>
tickDuration is now used to cycle through the poll queue. Signed-off-by: Peter Neuroth <[email protected]>
We remove the potentially heavy-load functions that ask to send out messages in parallel. Signed-off-by: Peter Neuroth <[email protected]>
We listen for peers to connect or disconnect to the node running peerswap and Add or Remove them from the poll service accordingly Signed-off-by: Peter Neuroth <[email protected]>
This is mainly to make the tests more stable as we don't have blocktime lag in regtest scenarios. But it does not hurt to have the extra check in real life also. Signed-off-by: Peter Neuroth <[email protected]>
We only want to add the peer to the queue if we do not know about it already. Signed-off-by: Peter Neuroth <[email protected]>
Testing: I want to see what happens to the timing of polls if networking is delayed by 10 seconds. In that case does 10 polls happen simultaneously or does it happen one at a time separated by at least 1 second between each poll? |
It finally disappeared about 5 minutes ago for me running 725ca2c. The patches at least seem to have extended the amount of time it takes for the node to disappear in some cases? In the past it would only take 3-4 hours to drop from |
Summary of discussion
|
Blockstream Store experienced another curious problem where an incoming swap-in attempt got stuck in state For the moment I recommend not testing #211. It needs work. |
|
We had some trouble on large nodes sending out all poll messages (see #185), maybe because we tried to send them out concurrently. This PR updates the polling service to use a sequential, time-ordered model to send out the poll messages.
Therefore we add a queue which elements are time ordered and by time we mean the timestamp at which we should send the next message to the peer.
Our main loop now is tick based and checks the following every other second:
This way we reduce the stress that we put on the node and the network connections.
If this was the cause for the disappearing peers, this resolves #185