deploy: the VM router was a black hole; replaced pods cost the Python SDK 45s each - #553
Conversation
The GCE VM router sam-router-<env>-vm has been a black hole since it was added: Container-Optimized OS drops inbound traffic on the host by default (INPUT policy DROP, ssh excepted), so the VPC rule allow-sam-hub-p2p opened tcp/udp 4501 to a host that never let a SYN through. The router enrolled, so the control plane listed it and every member dialed it on join, paying a dial timeout for nothing: 5s for a sam-node, 10s for the JS SDK, 15s for the Python SDK, which paid it again on every connect() to a peer none of the reachable routers relayed for. Verified on bananas: with the two iptables rules added by hand the port answered at once, and a Python caller's join went from 22s to 8s. The startup script runs on every boot, so the rules do not need to persist.
Measured in the bananas canary namespace with agent_mesh at DEBUG: one provider record naming a pod a rollout had replaced cost connect() 45s. Its address answered nothing (15s); the three admitted routers refused the circuit at once; the DHT named the same address, which was dialed again (15s); and the router the control plane lists that the member had not joined through was admitted on its own (15s). Routers keep a provider record for 48 hours by default and a deploy replaces both everything pods, so a caller meets several such records, and the sam-sdk-python probe ran out its 120s budget on them. connect() now dials the peer's own addresses and the circuit through every admitted router at once, then, for a peer none of them reached, every path the DHT and the control plane add, at once: the DHT's addresses this caller has not dialed yet, and the routers not joined through, each admitted on the way. A dead peer costs two dial timeouts, one per step, however many routers there are. The test pins the 45s case at 30s with the same inputs.
A router keeps a DHT provider record for 48 hours by default, and every bananas deploy replaces both everything pods, so callers of mcp://everything meet the dead pods of the last two days of deploys before a live one, each costing a dial timeout. Nodes reprovide every five minutes; an hour on bananas outlives a dozen missed reprovides and clears a replaced pod within the hour. Hub keeps the library default: it deploys from release tags, rarely. --dht-provider-addr-ttl is rendered into the router StatefulSet and the VM router from one per-environment value in deploy.yaml.
![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 optimizes the peer connection process in the Python SDK by dialing a peer's direct addresses and relayed paths through admitted routers concurrently, reducing connection latency and dial timeouts when dealing with replaced peers. It also exposes the DHT provider address TTL configuration in the Kubernetes router template and E2E tests. Feedback on the changes points out a potential issue where concurrent dialing of unjoined routers could lead to duplicate admissions of the same router if multiple multiaddrs point to the same peer ID, and suggests deduplicating relays by their peer ID before dialing.
| tried = {str(a) for a in direct} | ||
| routed, relays = await self._routed_addresses(target) | ||
| routed = [a for a in routed if str(a) not in tried] | ||
| for addr in self._unjoined_routers(target): | ||
| if not any(str(r) == str(addr) for r in relays): | ||
| relays.append(addr) | ||
| if await self._connect_through(target, routed, [], relays, failures): | ||
| return target |
There was a problem hiding this comment.
The new concurrent dialing of unjoined routers in _connect_through can lead to concurrent duplicate admissions of the same router if relays contains multiple different multiaddrs pointing to the same peer ID (e.g., an IP address and a DNS address for the same router). This results in redundant network connections and duplicate entries in self.routers.
Deduplicating relays by their peer ID (using info_from_p2p_addr(addr).peer_id) before passing them to _connect_through ensures that each unique router is only admitted once.
tried = {str(a) for a in direct}
routed, raw_relays = await self._routed_addresses(target)
routed = [a for a in routed if str(a) not in tried]
seen_relays = set()
relays = []
for addr in raw_relays + self._unjoined_routers(target):
try:
pid = str(info_from_p2p_addr(addr).peer_id)
if pid not in seen_relays:
seen_relays.add(pid)
relays.append(addr)
except Exception:
pass
if await self._connect_through(target, routed, [], relays, failures):
return target
1 participant
The
sam-sdk-pythoncold-path probe kept failing the bananas deploy (hung for 120s, 1 attempt) after #546, while JS and Go passed. Reproduced in the canary namespace with a one-off Job runningexamples/call.pywithagent_meshat DEBUG:Three causes, one commit each:
The GCE VM router (
sam-router-bananas-vm) has been unreachable since it was added. Container-Optimized OS drops inbound traffic on the host by default (-P INPUT DROP, ssh excepted); the VPC ruleallow-sam-hub-p2popens 4501 to a host that never let a SYN through. The router enrolls, so/infolists it as a fourth router and every member dials it: sam-node pays its 5s connect timeout at join, JS 10s, Python 15s at join and again on everyconnect()that falls through to the routers it has not joined through. The startup script now adds the twoiptablesrules beforedocker run. Applied by hand on the running VM to verify: the port answered at once and the Python join went from 22s to 8s.The Python SDK dialed the same dead address twice and admitted unjoined routers one at a time.
connect()now dials the peer's own addresses and the circuit through every admitted router at once, then everything the DHT and the control plane add that has not been tried, at once. A dead peer costs two dial timeouts, one per step, however many routers there are. New unit test pins the observed inputs at 30s; it reads 45s on the previous code.Routers keep a DHT provider record for 48h (library default), and every bananas deploy replaces both
everythingpods, so callers meet the dead pods of two days of deploys.--dht-provider-addr-ttlis now rendered from one per-environment value in deploy.yaml:1hon bananas (nodes reprovide every 5m),0s= library default on hub. Both the StatefulSet and the VM router get it.Also noted but not changed here:
deploy.yamlstill passes--bootstrap-tokento the VM router as a flag value inside instance metadata (AGENTS.md says secrets travel by path or environment).Verified:
make sdk-python(97 passed);TestNativeSDKsMesh,TestNativeSDKsAcrossRouters,TestNativeSDKs,TestNativeSDKA2A,TestNativeSDKExamples,TestSDKCanaryScript,TestStandaloneSDKAgents*pass with both SDK toolchains; the router template renders with the new variable and the VM startup-script argument parses (bash -n) with the flag as its last line.