Repository navigation
daemon: relayed trust handshakes complete in seconds instead of two minutes - #500
Merged
Merged
Conversation
TeoSlayer
force-pushed
the
feat/fast-relayed-handshake
branch
from
October 1, 2026 18:56
0663a08 to
29e9b2a
Compare
Collaborator
Author
|
Rebased onto current |
…inutes Between two private nodes the handshake request and its answer are parked at the registry and each side learns of them only by polling, once per keepalive interval (60s): ~59s for the request to show up on the target, ~57s more for the approval to reach the requester. Keep the 60s poll as the baseline and add three bounded triggers (handshakepoll.go): - After sending a handshake request the node polls every 2s until the peer answers or becomes trusted, for at most 2 minutes. Automatic (dial-driven) requests cannot restart the window for the same peer within 10 minutes. - The IPC handlers behind pending / trust / approve / reject / wait-for-trust poll first, at most once per 2s and bounded to 3s when the registry is slow. - A beacon notify ([0x0A][kind], accepted only from the beacon's address) triggers one poll from a token bucket (burst 3, one per 5s), never sooner than 2s after the previous poll. Released beacons do not send it yet. One extra timer in the existing poll loop, armed only while a request is outstanding or a notify is owed a poll; no goroutine per request. The loop's startup jitter no longer delays these triggers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestHandshakeTrustPersistence and TestHandshakeTrustLoadVerify stat/read trust.json the moment the peer shows up in the trusted list. The handshake plugin marks the peer trusted in memory and writes the store just after, so the tests depended on losing that race. The trust-list request now does a registry poll first, which lined the read up with the gap every time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llows the one deferred poll A burst of pokes is served by one poll at once and, if any poke lands after that poll began, one more after handshakeOnDemandGap. The test required exactly one and failed in CI when the scheduler let part of the burst land late (registry polls = 2, want 1). It now checks the bound that holds: one or two polls for the burst, and none after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
force-pushed
the
feat/fast-relayed-handshake
branch
from
October 2, 2026 09:44
29e9b2a to
e3d54a0
Compare
…poll Fixes from an independent review of this branch. - An on-demand poll gave up after 3s but had already asked the registry, which empties the node's handshake inbox as it answers: a reply that came later was thrown away, and with it the requests and approvals it carried. A poll now runs on its own goroutine and always completes; a caller only stops waiting for it. - At most one poll is in flight. A caller that arrived behind a stuck poll used to wait 3s and then start another with its own 3s; now it waits for the one in flight, counted from when that poll started, and starts nothing. One slow registry call occupies one pooled connection, not one per caller. - Wait-for-trust polled before checking trust. pilotctl calls it with a zero timeout before every send, connect and ping, so a busy node made up to 30 polls a minute with no handshake in flight and stalled 3s per command when the registry hung. It polls only while a request this node sent that peer is unanswered. - fastActive asked the handshake plugin about trust with the scheduler's lock held; the plugin can hold its own lock across a registry lookup and the tunnel read loop takes the scheduler's lock for every notify. The question is now asked with the lock released. - A beacon notify must be exactly [0x0A][0x01]; a bare type byte or an unknown kind no longer triggers a poll. - The extra timer polls only if a poke or an unanswered request is still owed one, and timer-driven polls keep to the 2s gap. - Peers kept only for their rearm time no longer fill the 64-entry table and deny a new request its fast polling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
pushed a commit
that referenced
this pull request
Oct 7, 2026
#500 made TestHandshakeTrustLoadVerify and TestHandshakeTrustPersistence wait up to 2s for trust.json to exist. The handshake manager writes the file from a goroutine after the in-memory state changes, and its first write can still hold only the pending request, so LoadVerify could read a file with no trusted peer ("trust file should have at least one trusted peer"). It now waits for a write that lists one, and both tests allow 5s, which is what failed under load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
TeoSlayer
added a commit
that referenced
this pull request
Oct 7, 2026
… appears (#508) tests: wait for a trust file that lists the peer, not just for the file #500 made TestHandshakeTrustLoadVerify and TestHandshakeTrustPersistence wait up to 2s for trust.json to exist. The handshake manager writes the file from a goroutine after the in-memory state changes, and its first write can still hold only the pending request, so LoadVerify could read a file with no trusted peer ("trust file should have at least one trusted peer"). It now waits for a write that lists one, and both tests allow 5s, which is what failed under load. Co-authored-by: Teo Calin <calinteodor@Teos-MacBook-Pro.local> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
pushed a commit
that referenced
this pull request
Oct 7, 2026
…-ups to #500 From the second review of #500. - Shutdown closed the registry client without waiting for a poll in flight. The registry empties the node's handshake inbox as it answers, so a restart during a poll lost what it carried. doStop now waits up to 5s for the poll (waitIdle), and pollHandshakes starts none once the daemon is stopping. - At the 64-peer cap, only the closed-window peer whose rearm ends soonest is evicted; the others keep their hold-off against automatic handshakes reopening a window. - A beacon notify with bytes after the handshake kind is acted on, so a later beacon can extend the message without older daemons ignoring it. - Comments: the package doc describes the wait-for-trust condition, and pollRelayedHandshakes no longer documents the removed timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
added a commit
that referenced
this pull request
Oct 7, 2026
…500 (#514) * daemon: shutdown waits for a handshake poll in flight; smaller follow-ups to #500 From the second review of #500. - Shutdown closed the registry client without waiting for a poll in flight. The registry empties the node's handshake inbox as it answers, so a restart during a poll lost what it carried. doStop now waits up to 5s for the poll (waitIdle), and pollHandshakes starts none once the daemon is stopping. - At the 64-peer cap, only the closed-window peer whose rearm ends soonest is evicted; the others keep their hold-off against automatic handshakes reopening a window. - A beacon notify with bytes after the handshake kind is acted on, so a later beacon can extend the message without older daemons ignoring it. - Comments: the package doc describes the wait-for-trust condition, and pollRelayedHandshakes no longer documents the removed timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * daemon: shutdown waits for the handshake poll before the manager stops Review fixes. - Shutdown waited for a poll in flight only after the handshake manager had stopped, so the poll's reply was processed by a stopped manager: requests and approvals were neither saved nor answered, and the registry had already emptied the inbox. The wait now comes before the manager stops. - No poll can start once shutdown has begun: the scheduler is closed under the same lock that starting a poll takes. Before, a caller past the stopping check could start one after the wait had returned. - The wait shares the 5s deadline shutdown already gives its background goroutines instead of adding 5s of its own. - The shutdown test now calls Stop and checks the order; tests cover the closed scheduler and which settled peer is let go. Each fails against the matching revert. - Changelog entries for the eviction and notify changes; the notify comments say v1.16.0 still requires exactly two bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Teo Calin <calinteodor@Teos-MacBook-Pro.local> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Pull Request
Summary
Trust between two private nodes took about two minutes. A requester cannot resolve a private node before trust exists, so the handshake is relayed through the registry, and each side only found the relayed message in
handshakePollLoop, which ticks every 60 seconds: ~57s for the request to reach the target, ~59s for the approval to come back. The direct path (to a public node) was already instant.Changes
pending,trust,approve,rejectand wait-for-trust check the registry first, at most once per 2s (3s bound if the registry is slow), so an operator asking "anything pending?" sees a relayed request at once.[0x0A][0x01]from the beacon's address triggers a poll. It needs beacon: NotifyNode tells a node the registry holds a relayed handshake for it beacon#59 and registry: tell the beacon which node a relayed handshake was parked for rendezvous#129 on the server side; without them nothing sends it.Load
The 60-second baseline is unchanged: two idle nodes made 32 registry requests in 240s before and after. Worst-case handshake polls per node per minute (today: 1):
pendingA notify never triggers a poll sooner than 2s after the previous one (deferred, not dropped). Automatic, dial-driven handshakes cannot restart the fast window for the same peer within 10 minutes.
Measured (Docker, private requester and target, manual approval)
pendingThe un-updated side keeps working at the old pace in every mix. Baseline approval time is either ~57s or near zero depending on how the two nodes' 60s ticks fall.
Notes for review
TestHandshakeTrustPersistenceandTestHandshakeTrustLoadVerifyreadtrust.jsonthe instant the peer appears in the trusted list, but the handshake plugin writes the file just after. The new poll-before-listing made that gap show every time; the tests now wait up to 2s for the file. Product behaviour is unchanged.TestRelayedHandshakeReachesUnattendedTargetskips until the beacon and rendezvous modules are bumped to releases with the notify.Test Plan
go build ./...,go vet ./..., unit suite withGOWORK=offTestBeaconNotifyOnlyFromBeacon;TestRelayedHandshakeCompletesInSeconds(fails on main: "relayed request not in the target's pending list after 20s")go test -parallel 4 -count=1 ./tests/: one failure,TestManualSnapshotTrigger, which binds fixed port 127.0.0.1:18080 held by another process on the test machineChecklist
go.mod/go.sumunchanged🤖 Generated with Claude Code