fix: reuse SSH transports for remote collectors - #77
Conversation
|
Independent confirmation of both the problem and this fix, from a two-node DGX Spark (GB10) cluster running Ubuntu, monitored by sparkDash on the head with one remote worker over key auth. I found the session churn while investigating something else, derived the same ~3.6/s figure from the call sites before finding #73, then measured it. Numbers below are from the worker. Before, on The daemon that grew for us was The head reads After pinning our submodule to a561c22 and redeploying, same measurement: The four tests in One negative result worth recording. I sampled Context for why this mattered to us beyond log noise: these are unified-memory boxes where the inference runtime reserves most of 121 GB, so host headroom is single-digit GB. 1.6 GB held by dbus was a meaningful share of what was left. I am not claiming this PR fixes that; the reservation was our own misconfiguration. But on a memory-constrained host the churn is not free. |
Concept ACK — SSH ControlMaster reuseProduction Readiness: NO-GO Real bug: remote collectors were burning a full SSH/PAM login per metric poll. Design is mostly right — salted digest isolation, password out of argv/ControlPath, ControlMaster=auto, opt-out via SSH_CONTROL_PERSIST_SECONDS=0, single-flight probe. Critical: mux state is not invalidated when a post-probe sshExec fails. A dead master before expiresAt can recreate the auth storm this PR exists to stop. Clear Happy to ACK after that. — Vincent |
# Conflicts: # CHANGELOG.md # server/collectors/ssh.js
|
Fixed the post-probe failure invalidation in Added regression tests covering those cases, including 12 concurrent recovery callers plus a late old failure. Also merged current main ( Validation: SSH suite 7/7, full suite 197/197, typecheck, build and diff checks all pass. The earlier unrelated test failures are fixed in current main. The failure-recovery change is ready for another review; the production measurements in the description remain explicitly attributed to the initial transport-reuse version. |
Summary
Remote collectors currently create a full SSH/PAM login for every metric poll. Reuse one authenticated OpenSSH transport per remote unit, while keeping each collector command on its own exec channel with the existing timeout and output limit.
ControlMaster=autoandControlPersist.0700directory. Passwords stay out of argv and socket paths.sshCommandSpecAPI for independent tunnel callers.SSH_CONTROL_PERSIST_SECONDSdefaults to 60 seconds, is capped at one hour, and accepts0to restore one connection per command. Key andsshpassauthentication are supported.Evidence
On a two-DGX-Spark deployment, the old collectors generated about 3.6 complete SSH/PAM login lifecycles per second on the worker. An on/off/on monitor test measured
polkitdanonymous RSS growth of 0.954 MiB/min, flat while monitoring was stopped, and 0.966 MiB/min after restart. A same-command-rate comparison measured 0.526 MiB/min with independent SSH connections and 0 with a multiplexed connection.Production validation of the initial transport-reuse change showed a 0.000 MiB/min anonymous-RSS slope in two observation windows; a cold start dropped from 56 SSH authentications to one. These observations support reducing session churn, not a claim to fix an internal polkit allocation bug. The post-probe failure recovery added during review is covered by regression tests; it has not been separately deployed in that production test.
OpenSSH reference: https://man.openbsd.org/ssh_config.5#ControlPersist
Validation
Updated against main at
e03b9d6, including its SSH tunnel helper and test fixes.npm test: 197/197 pass, including the local TCP tunnel tests (requires permission to bind loopback sockets).npm run typecheck: pass.npm run build: pass.git diff --check: pass.