feat: ensure dials are sorted based on their recent success #2164
Conversation
…dials-sorted-success
| addr.lastFailure *= 2 | ||
| } else { | ||
| addr.lastFailure = Date.now() |
There was a problem hiding this comment.
Something seems wrong here.
Is lastFailure a timeout value or a date?
If its a date, then multiplying it by 2 is wrong.
If its a timeout value, setting as the current date is wrong.
There was a problem hiding this comment.
The lastFailure value is a unix timestamp but it's being utilized as a value for ordering the list of multiaddrs, therefore exponentially increasing it so that it will be penalized even further to the bottom of the list seems a valid approach i.e. it could be any unique incremental numerical value as long as it is sequentially increasing to create an order within the list of addrs.
|
|
||
| try { | ||
| if (pendingDial.peerId != null) { | ||
| peer = await this.peerStore.get(pendingDial.peerId) |
There was a problem hiding this comment.
maybe this peerStore.get should happen in _updateAddressStatus and leave _updateAddressStatus signature alone?
I think it tracks more nicely to have the get+update in the "same place".
Also thats one less promise that has to be fulfilled before we get to dial.
There was a problem hiding this comment.
I take your point but when refactored that way a potential ERR_NOT_FOUND would be propagated in lieu of a consequent and perhaps more meaningful error such as a NO_RESERVATION error.
…dials-sorted-success
Title
feat: ensure dials are sorted based on their recent success
Description
Given that we are now tracking dials based on their
lastSuccessandlastFailurewe should ensure that they the are properly ordered before dialling, and subsequently update the peerStore with that metadata dependent on the outcome of that dial.This PR also removes the
AUTO_DIAL_PEER_RETRY_THRESHOLDsince we are no longer sorting dials on a per peer basis but rather on a peer multiaddr basis. This adds a retry mechanism that exponentially backoffs of dialling failed addresses per peer.Closes #2150
Closes #2114
Related #2090
Notes & open questions
Change checklist