diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ebeaec1..2d28841d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -296,6 +296,14 @@ Detailed per-release notes are on the unknown type; before it answered that no verifier was configured. ### Changed +- **`pilotctl send-message` asks the daemon for its `info` reply once, not + twice.** The auto-handshake read the daemon's `features` from a second + `info` request, although the command had fetched one a moment earlier for + its first-contact check. The reply lists every peer and connection: on a + node with 5,400 peers it is several hundred KB, and parsing it costs the + CLI 7.5 ms of CPU and 2.9 MB of allocations, more than the rest of a send. + The second request happened on every send to an agent in the trusted list + (`list-agents`, `pilot-mom`) and to any public peer not yet trusted. - **The tunnel socket asks the kernel for 4 MB buffers** in each direction instead of the default (about 200 KB on Linux). Every tunnel shares the one socket, and several streams sending at once overflowed it. The kernel caps diff --git a/cmd/pilotctl/firstcontact.go b/cmd/pilotctl/firstcontact.go index b97bddaf..52b6efb4 100644 --- a/cmd/pilotctl/firstcontact.go +++ b/cmd/pilotctl/firstcontact.go @@ -54,14 +54,21 @@ func daemonHasFeature(d *driver.Driver, feature string) bool { if daemonFeatureSet == nil { daemonFeatureSet = map[string]bool{} if info, err := d.Info(); err == nil { - for f := range featuresOf(info) { - daemonFeatureSet[f] = true - } + noteDaemonFeatures(info) } } return daemonFeatureSet[feature] } +// noteDaemonFeatures fills the feature cache from an info reply the +// command already holds, so daemonHasFeature does not ask for another. +// The reply lists every peer and connection: on a node with a few +// thousand peers it is several hundred KB, and parsing it costs pilotctl +// more CPU than the rest of a send. +func noteDaemonFeatures(info map[string]interface{}) { + daemonFeatureSet = featuresOf(info) +} + func featuresOf(info map[string]interface{}) map[string]bool { out := map[string]bool{} list, _ := info["features"].([]interface{}) diff --git a/cmd/pilotctl/main.go b/cmd/pilotctl/main.go index 153be469..7eab060f 100644 --- a/cmd/pilotctl/main.go +++ b/cmd/pilotctl/main.go @@ -5009,6 +5009,7 @@ func cmdSendMessage(args []string) { firstContact := false if info, err := d.Info(); err == nil { firstContact = !peerSessionUp(info, target.Node) + noteDaemonFeatures(info) // maybeAutoHandshake needs them next } // Auto-handshake to peers in the embedded trusted-agents list. diff --git a/cmd/pilotctl/zz_firstcontact_test.go b/cmd/pilotctl/zz_firstcontact_test.go index 52a9ffde..a09bbdf2 100644 --- a/cmd/pilotctl/zz_firstcontact_test.go +++ b/cmd/pilotctl/zz_firstcontact_test.go @@ -300,3 +300,40 @@ func TestAutoHandshakeKeepsHandshakeForOlderDaemon(t *testing.T) { t.Fatal("older daemon: the trusted-agent handshake request must still be sent") } } + +// A send asks the daemon for its info reply once. The features the +// auto-handshake checks come from the reply already fetched for the +// first-contact check: the reply lists every peer and connection, so on a +// busy node a second one cost more than the rest of the send. +func TestSendMessageAsksForInfoOnce(t *testing.T) { + sd := newStreamDaemon(t) + sd.useDaemonNoRegistry(t) + daemonFeatureSet = nil + t.Cleanup(func() { daemonFeatureSet = nil }) + sd.onJSON(tdCmdInfo, tdCmdInfoOK, `{"node_id":1,"features":["reply_window"]}`) + // Not trusted yet, so the auto-handshake goes on to check features. + sd.onJSON(tdCmdHandshake, tdCmdHandshakeOK, `{"trusted":false}`) + + out := captureStdout(t, func() { + withJSON(func() { + cmdSendMessage([]string{"0:0000.0002.BBE4", "--data", "hello"}) // 179172, a trusted agent + }) + }) + if !strings.Contains(out, `"status":"ok"`) { + t.Fatalf("send failed: %s", out) + } + sd.mu.Lock() + defer sd.mu.Unlock() + infos := 0 + for _, f := range sd.received { + if f[0] == tdCmdInfo { + infos++ + } + } + if infos != 1 { + t.Fatalf("info requests = %d, want 1", infos) + } + if !daemonFeatureSet["reply_window"] { + t.Fatal("features from the first info reply were not kept") + } +}