feat(pqm): opt-in findpeer fallback on dial fail#1156
Draft
lidel wants to merge 1 commit into
Draft
Conversation
when the first dial to a provider fails and a peer router is configured, ask peerrouting.FindPeer once and retry the dial with the returned addrinfo. only retry if FindPeer surfaced at least one address that wasn't already in the routing-record set we just tried; redialing the same broken set would just burn another round. opt-in via the new WithPeerRouting option; default behavior is unchanged. originally surfaced by ipfs/service-worker-gateway#1067, where rainbow's bitswap host could not reach a peer the dht-side view already knew how to find.
lidel
changed the title
lidel
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1156 +/- ##
==========================================
+ Coverage 63.15% 63.23% +0.07%
==========================================
Files 267 267
Lines 26867 26886 +19
==========================================
+ Hits 16969 17002 +33
+ Misses 8173 8162 -11
+ Partials 1725 1722 -3
... and 9 files with indirect coverage changes 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In some setups where DHT is semi-separate from provider lookup, when a provider's routing-record
AddrInfowas stale or thin, the single dial inProviderQueryManagerfailed and the provider was discarded, even when the DHT already knew reachable addresses for that peer.Surfaced in ipfs/service-worker-gateway#1067, where rainbow's bitswap host could not connect to peers the DHT-side view could.
How this helps
This PR upstreams the idea from ipfs/rainbow#372 and adds opt-in
WithFindPeerFallback(peerRouter)torouting/providerquerymanager. When enabled, on dial failure the manager callsFindPeeronce and retries the dial with the returnedAddrInfo, but only ifFindPeersurfaced at least one address that wasn't in the set just tried. Default behavior is unchanged; wire with a DHT client to enable.Note
ipfs/rainbow#372 perfectly fixes the problem from ipfs/service-worker-gateway#1067, kubo and other users of kad-dht are not imacted this, it was specific to special setup Rainbow uses.
This PR is just an idea how we could make the opt-infix generic. Ok to decide to not merge this if this does not feel useful outside of Rainbow.