Skip to content
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

[network] Delay peer outbound connection in ProtocolController if the dial failed #494

Open
dmitry-markin opened this issue Jul 27, 2023 · 1 comment
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@dmitry-markin
Copy link
Contributor

Currently, if the dial fails but the peer has high reputation, it's picked up as outbound connection candidate again, and dial is repeated on the next slot allocation (within a second). It would be nice to not dial such peers immediately.

Not super critical, but something to look into someday.

@dmitry-markin dmitry-markin changed the title Delay peer outbound connection in ProtocolController if the dial failed [network] Delay peer outbound connection in ProtocolController if the dial failed Jul 27, 2023
@altonen
Copy link
Contributor

altonen commented Aug 2, 2023

We need some additional considerations for this whole behavior.

If the dial fails, the address is considered undialable without any further testing. We should allow something like three failed dial attempts before the address is considered not working and can be removed. These dial attempts should not happen back to back because the node may be temporarily unavailable and later reachable again from the same address.

Right now the addresses are removed after one failed dial attempt which means some of the peers in PeerStore have no known addresses but they're still selected for an outbound connections. The dial will of course fail for these nodes but ProtocolController is not informed about this absence of known addresses which is a problem. DiscoveryBehavior should have the ability to remove known peers from PeerStore if it detects there are no available addresses for the peer.

@altonen altonen transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. and removed I4-annoyance labels Aug 25, 2023
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
Status: Backlog 🗒
Development

No branches or pull requests

3 participants