Improve sync reliability #133
Conversation
|
Also increased the protocol timeouts from 5000 to 8000. This is somewhat too often triggered and should be no problem to increase on such a level without side effects. |
|
Here is a screenshot with the "Looking for suited peers..." message:
|
|
I think with this PR we should be back to the situation on having a satisfactory and repeatedly reliable syncing experience on at least one network. 😄 |
|
Ok, added one last thing which is a bootstrap retrigger after two periods with no peers. This to some extend solves the all-or-nothing problem being that there is often the situation that chain sync either starts relatively quickly or not at all. Here is a screenshot on its behavior:
|
|
Hmm, the tests won't terminate here anymore. @ryanio could you have a short look here if I use async/await correctly? |
|
(or could it be by this |
|
taking a look, I will add some logic to clearInterval on close. The integration tests still aren't terminating. I found a few open()s without close() in the tests, but the ones I've found haven't fixed it so far. |
add pool.close() to destroy
to indicate that it is a private property
|
i got it! i noticed the peerpool wasn't properly being closed in the integration tests so i was able to fix it with 3672296. |
| if (this.size === 0) { | ||
| this.noPeerPeriods += 1 | ||
| if (this.noPeerPeriods >= 3) { | ||
| const promises = this.servers.map(async server => { |
There was a problem hiding this comment.
@ryanio Thank you
I don not know this context in detail yet.
Which do you think is better, Promise.allSettled or Promise.all?
There was a problem hiding this comment.
It's okay if it looks good as it is
There was a problem hiding this comment.
@ryanio let me know if I need to update anything here or you can alternatively also directly push yourself
@tomonari-t thanks for reviewing!
There was a problem hiding this comment.
@tomonari-t ah thanks. I think Promise.all looks good since Promise.allSettled seems like it's useful if we need to use the results of the outcomes. In these two cases I think we're just interested in waiting for them to finish.
This PR improves sync reliability by increasing the timeout for jobs in the
Fetcherclass from 5000 to 8000.With the existing timeout peers where directly banned again - especially during startup and first initialization period, see:
This led to banning of especially valuable bootstrap peers which made initial connection start often difficult respectively unsuccessful.
This effect is in particular noticeable on a
rinkebyconnection (--network rinkeby) which generally is the most suited network atm for a reliable connection.If generally no suited peers are found on a network, this change won't help on the situation either.
I also added a simple repeated logger with a "Looking for suited peers..." message. This is psychologically helpful as some feedback loop when getting an initial connection is difficult and takes some time (often occuring on
mainnete.g.). This gives therefore a "Something is happening..." notion and gives a clear feedback (every 20 seconds) that the client has not crashed or stopped working.This helps a lot to "bridge" the first 1-2 minutes, if it takes that long until a connection succeeds.