sdk: a member follows a router that came back on another address - #554
Conversation
…e holds Seen on hub after GKE replaced every node on 2026-09-30: the router pods were rescheduled with their keys and new IPs, and the Python agent in the canary pod logged "relay reservation on router ... not renewed, retrying in 30s: no connection ... within 15s" every 30s for five hours. The JS caller beside it failed every call with "All multiaddr dials failed" and PERMISSION_DENIED, since the agent advertised a relay it was no longer connected to. The reservation loop did resolve the router's /dnsaddr name again and got the new address. host.connect appends that address to the peerstore, behind the ones identify learned from the old pod, and py-libp2p's dial_peer selects one address per transport, the first in that list: the old pod IP, a black hole, on every retry, for as long as the record lived, and every connect renewed its TTL. A refusal instead of a black hole would have put the peer in the swarm's negative cache, keyed by peer, for a minute more. dial() now drops what the peerstore holds for the peer and takes it out of the negative cache before connecting, so the addresses a caller just resolved or was handed by a record are the ones dialed. Every dial the SDK makes goes through it: admission at join, re-reservation, a provider's direct addresses, a router met through the DHT, and the DHT walk itself. Two tests: dial() against a real host whose peer was last seen on a port nobody answers on and whose swarm holds the failed dial, and a session whose router comes back with the same key on another address, the name now resolving there; on the previous code the member reserved on the old address again.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the dial function in host.py to clear a peer's cached addresses and unblock them in the swarm before attempting a connection, resolving issues where rescheduled pods with new IPs are dialed at stale addresses. It also adds corresponding unit and integration tests to verify this behavior. The review feedback suggests adding a defensive None check for host.get_peerstore() to prevent potential AttributeErrors, and bounding the wait_for helper in tests with a timeout to avoid indefinite hangs.
| negative cache is keyed by peer, not by address, and would refuse the new | ||
| address for a minute after the old one failed, so the peer is taken out | ||
| of it as well.""" | ||
| host.get_peerstore().clear_addrs(info.peer_id) |
There was a problem hiding this comment.
To prevent potential AttributeError or NoneType errors if get_peerstore() returns None (for instance, in custom mock hosts or uninitialized states), we should add a defensive None check before calling clear_addrs. This aligns with defensive programming practices for nullable references.
| host.get_peerstore().clear_addrs(info.peer_id) | |
| peerstore = host.get_peerstore() | |
| if peerstore is not None: | |
| peerstore.clear_addrs(info.peer_id) |
| async def wait_for(predicate): | ||
| while not predicate(): | ||
| await trio.sleep(0.1) |
There was a problem hiding this comment.
The helper function wait_for currently loops indefinitely if the predicate never becomes true, causing the test to hang until the outer 60-second timeout cancels it. Bounding the wait with a native Trio timeout (e.g., 5 seconds) allows the test to fail fast and provides a clearer failure context.
| async def wait_for(predicate): | |
| while not predicate(): | |
| await trio.sleep(0.1) | |
| async def wait_for(predicate, timeout=5.0): | |
| with trio.fail_after(timeout): | |
| while not predicate(): | |
| await trio.sleep(0.1) |
aojea
changed the title
1 participant
Root cause of the
hubTestnet Health failure (run 36719875195,Canaries: python-agent-canary-hub), and the gap next to it. Not the dedup case discussed on #553.GKE replaced every node of the cluster on 2026-09-30 08:27–08:53; the router pods were rescheduled with their keys and new IPs. The Python agent (rc.7) in the canary pod has logged this every 30s since:
10.84.4.91is the router's old pod IP. With no reservation the agent's advertised circuit addresses are dead, which is what the JS caller in the same pod reports (All multiaddr dials failed,PERMISSION_DENIED).Two commits:
sdk/python: dial a peer at the addresses given, whatever the peerstore holds._reserve_againre-resolves/dnsaddr/bootstrap.hub.sam-mesh.dev/p2p/…and gets the new address, buthost.connectonly appends it to the peerstore behind the addresses identify learned from the old pod (and resets their 120s TTL on every call), and py-libp2p 0.8Swarm.dial_peerselects one address per transport class, the first in that list. The stale pod IP black-holes forDIAL_TIMEOUT, the loop retries, same address, forever. A refusal instead of a black hole would additionally put the peer in the swarm's negative cache (keyed by peer, 60s).dial()now clears the peerstore's addresses for the peer and takes it out of the negative cache beforehost.connect; every dial the SDK makes goes through it. The same path affects any peer that keeps its key across a move, so a sam-node pod with a--keys-pathvolume too.sdk: reserve again on a router where the control plane lists it now. A mesh that hands out literal addresses (sam-one, a kind cluster, the integration harness) has no name to re-resolve: a router that came back elsewhere is known only through the control plane's list, which every pull refreshes, and both SDKs' relay upkeep dialed the address admitted at join. The Python reservation loop and the JSkeepRelaynow dial the router at the addresses the credential lists for its peer ID (the admitted address only when the list no longer names it) and keep the address they reached it on, soconnect()'s circuits through that router and the Python member's advertised relay address follow.Tests, each failing on
mainand each half of thedial()fix needed:test_a_peer_that_came_back_on_another_address_is_dialed_there(real py-libp2p hosts): the peerstore holds a port nobody listens on and the swarm a failed dial to it;dial()at the current address connects. Fails onmainwithrecently failed all addresses (negative cache), and withno addresses established a successful connectionwhen only the cache is cleared.test_a_router_that_came_back_on_another_address_is_reserved_on_again(Python): two routers with one key, member joined by/dnsaddrname, the name re-pointed and the connection dropped. Fails onmainwithdialed the old pod's address again.test_a_router_the_control_plane_lists_elsewhere_is_reserved_on_there(Python) anda router the control plane lists elsewhere is reserved on there(JS): literal addresses, the member pulls the list naming the second router, connection dropped. Fail withdialed the address admitted at join again/ an empty relay address list.TestNativeSDKsFollowAMovedRouter(Go integration, ~8s after the fixture): restarts asam-routeron another port with its key under a Python and a JS member, has each pull the list, waits for its relayed address to name the new port, then reaches each member through it from a Go peer. Fails on the previous SDKs withjs still advertises [], want the router at ….SAM_SDK_RELAY_CHECK_SECONDSlets the conformance runners check their reservation often enough.Verified: Python 100 passed; JS 89 passed,
buildandexamples;TestNativeSDK*,TestSDKCanaryScript,TestStandaloneSDKAgents*pass with both runners rebuilt;golangci-lintontests/integrationclean;gen-sdk-docs -checkclean.