diff --git a/server/collectors/SystemCollector.js b/server/collectors/SystemCollector.js index 2c2c9d05..c3ae68e9 100644 --- a/server/collectors/SystemCollector.js +++ b/server/collectors/SystemCollector.js @@ -1178,6 +1178,12 @@ export class SystemCollector { "echo '---'", // WoL MAC for the primary LAN NIC on DGX Spark `cat /sys/class/net/${WOL_INTERFACE}/address 2>/dev/null || true`, + "echo '---'", + // Link speed for every interface, not just the primary one: which + // interface is primary only falls out of the route table above, and + // fetching that one afterwards cost a second SSH login per poll. + // Virtual interfaces have no `speed`; they just come back blank. + "for d in /sys/class/net/*/speed; do echo \"$(basename $(dirname $d)):$(cat $d 2>/dev/null)\"; done 2>/dev/null || true", ].join("; "); const output = await sshExec(this.spark, cmd); @@ -1187,6 +1193,16 @@ export class SystemCollector { const ipOut = sections[2]?.trim() || ""; const operstateOut = sections[3]?.trim() || ""; const wolMac = normalizeMac(sections[4]?.trim() || ""); + const speedOut = sections[5]?.trim() || ""; + + // Parse link speed lines ("enP7s7:10000"); blank values stay unknown. + const speedMap = new Map(); + for (const line of speedOut.split("\n")) { + const idx = line.indexOf(":"); + if (idx <= 0) continue; + const mbps = parseInt(line.slice(idx + 1).trim(), 10); + if (Number.isFinite(mbps) && mbps > 0) speedMap.set(line.slice(0, idx), mbps); + } // Parse operstate lines ("enP7s7:up") const operstateMap = new Map(); @@ -1256,22 +1272,7 @@ export class SystemCollector { primaryInterface = alt?.name ?? primaryInterface; } - let linkSpeedMbps = null; - if (primaryInterface) { - try { - // Interface name is from the kernel; still keep it to safe chars - if (/^[a-zA-Z0-9._-]+$/.test(primaryInterface)) { - const speedRaw = await sshExec( - this.spark, - `cat /sys/class/net/${primaryInterface}/speed 2>/dev/null || true` - ); - const n = parseInt(String(speedRaw).trim(), 10); - if (Number.isFinite(n) && n > 0) linkSpeedMbps = n; - } - } catch { - /* link speed optional */ - } - } + const linkSpeedMbps = (primaryInterface && speedMap.get(primaryInterface)) || null; return { primaryInterface, linkSpeedMbps, interfaces: tagged, wolMac }; } catch (err) { diff --git a/server/collectors/__tests__/ssh.test.js b/server/collectors/__tests__/ssh.test.js index 4d209c1b..8798c6b4 100644 --- a/server/collectors/__tests__/ssh.test.js +++ b/server/collectors/__tests__/ssh.test.js @@ -40,3 +40,21 @@ test("sshCommandSpec: missing user throws", () => { /SSH config missing/ ); }); + +test("sshCommandSpec: commands share one master connection", () => { + const spec = sshCommandSpec(keySpark, { remoteArgv: ["cat /proc/uptime"] }); + const dash = spec.args.indexOf("--"); + const master = spec.args.indexOf("ControlMaster=auto"); + const controlPath = spec.args.find((a) => a.startsWith("ControlPath=")); + const persist = spec.args.find((a) => a.startsWith("ControlPersist=")); + assert.ok(master >= 0 && master < dash); + assert.ok(controlPath && controlPath.endsWith("sparkdash-%C")); + assert.equal(persist, "ControlPersist=300"); +}); + +test("sshCommandSpec: multiplex:false opts out (tunnels own their connection)", () => { + const spec = sshCommandSpec(keySpark, { multiplex: false, extraSshArgs: ["-N"] }); + assert.ok(spec.args.includes("ControlMaster=no")); + assert.ok(spec.args.includes("ControlPath=none")); + assert.ok(!spec.args.includes("ControlMaster=auto")); +}); diff --git a/server/collectors/llmTunnel.js b/server/collectors/llmTunnel.js index d4d94a7e..70900b2c 100644 --- a/server/collectors/llmTunnel.js +++ b/server/collectors/llmTunnel.js @@ -151,6 +151,10 @@ export async function openSshLlmTunnel(spark, remotePort, opts = {}) { opts.onStatus?.(`Opening SSH tunnel to 127.0.0.1:${p}…`); const spec = sshCommandSpec(spark, { + // A forward has to live on its own connection: killing this process is how + // the tunnel gets torn down, and a channel on a shared master would outlive + // it. + multiplex: false, extraSshArgs: [ "-N", "-o", diff --git a/server/collectors/ssh.js b/server/collectors/ssh.js index 56b0967e..59c97d34 100644 --- a/server/collectors/ssh.js +++ b/server/collectors/ssh.js @@ -7,7 +7,15 @@ */ import { execFile } from "child_process"; import fs from "fs"; -import { COMFY_PORT, COMFY_PROBE_TIMEOUT_MS, SSH_CONNECT_TIMEOUT } from "../config.js"; +import os from "os"; +import path from "path"; +import { + COMFY_PORT, + COMFY_PROBE_TIMEOUT_MS, + SSH_CONNECT_TIMEOUT, + SSH_CONTROL_PERSIST, + SSH_MULTIPLEX, +} from "../config.js"; import { isAllowedTargetHost, isValidSshUser } from "../validate.js"; import { llmProbeHost } from "./llmHost.js"; @@ -54,6 +62,36 @@ function sshpassAvailable() { return _sshpassAvailable; } +/** + * `-o` flags that let every poll ride an already-authenticated connection. + * + * Without them each collector tick pays for a fresh TCP connect, key exchange + * and authentication. At the default cadence a single remote Spark takes 217 + * of those per minute, and on password auth the KDF alone dominates the cost — + * the login is far more expensive than the `cat /proc/meminfo` it carries. + * With a shared master, the first command connects and the rest open a channel + * on the socket that is already up. + * + * `%C` hashes (local host, user, host, port) into a fixed-length name, so the + * socket path can never grow past the ~104 byte sun_path limit no matter how + * long the hostname is. A master that died leaves a stale socket behind; + * `ControlMaster=auto` notices, reconnects, and replaces it. + * + * @returns {string[]} + */ +function multiplexOpts() { + if (!SSH_MULTIPLEX) return ["-o", "ControlMaster=no", "-o", "ControlPath=none"]; + const controlPath = path.join(os.tmpdir(), "sparkdash-%C"); + return [ + "-o", + "ControlMaster=auto", + "-o", + `ControlPath=${controlPath}`, + "-o", + `ControlPersist=${SSH_CONTROL_PERSIST}`, + ]; +} + /** * Build file/args/env for an ssh (or sshpass) invocation. No shell interpolation. * @@ -61,8 +99,12 @@ function sshpassAvailable() { * options and before `-- user@host`. `remoteArgv` is the remote command (omit * for `-N` tunnels). * + * Pass `multiplex: false` for invocations that need a connection of their own + * — a `-N` port forward has to own its channel so that killing the process + * tears the forward down with it. + * * @param {object} spark - * @param {{ extraSshArgs?: string[], remoteArgv?: string[] }} [opts] + * @param {{ extraSshArgs?: string[], remoteArgv?: string[], multiplex?: boolean }} [opts] * @returns {{ file: string, args: string[], env: NodeJS.ProcessEnv, targetHost: string }} */ export function sshCommandSpec(spark, opts = {}) { @@ -89,6 +131,9 @@ export function sshCommandSpec(spark, opts = {}) { `ConnectTimeout=${SSH_CONNECT_TIMEOUT}`, "-o", "StrictHostKeyChecking=accept-new", + ...(opts.multiplex === false + ? ["-o", "ControlMaster=no", "-o", "ControlPath=none"] + : multiplexOpts()), ]; const remote = `${user}@${targetHost}`; diff --git a/server/config.js b/server/config.js index 2bbf2aac..649576f0 100644 --- a/server/config.js +++ b/server/config.js @@ -24,6 +24,15 @@ const LLM_PROBE_TIMEOUT_MS = 3000; const COMFY_PROBE_TIMEOUT_MS = parseInt(process.env.COMFY_PROBE_TIMEOUT_MS || "3000", 10); const TAILSCALE_PROBE_TIMEOUT_MS = parseInt(process.env.TAILSCALE_PROBE_TIMEOUT_MS || "8000", 10); const SSH_CONNECT_TIMEOUT = 5; // seconds +// Reuse one authenticated SSH connection per Spark instead of dialing a new one +// for every collector tick. Set SSH_MULTIPLEX=0 to go back to one connection +// per command (e.g. an sshd with `MaxSessions 1`). +const SSH_MULTIPLEX = process.env.SSH_MULTIPLEX !== "0"; +// How long an idle master connection lingers, in seconds. Long enough that the +// slowest loop (Hermes, 10 min) still finds it up would keep a socket open for +// hours; 5 minutes covers every metric domain and lets a rebooted Spark drop +// its socket quickly. +const SSH_CONTROL_PERSIST = process.env.SSH_CONTROL_PERSIST || "300"; // ─── Poll intervals (milliseconds) ─────────────────────── const POLL_INTERVAL_GPU = parseInt(process.env.POLL_INTERVAL_GPU || "2000", 10); @@ -101,6 +110,8 @@ export { COMFY_PROBE_TIMEOUT_MS, TAILSCALE_PROBE_TIMEOUT_MS, SSH_CONNECT_TIMEOUT, + SSH_MULTIPLEX, + SSH_CONTROL_PERSIST, POLL_INTERVAL_GPU, POLL_INTERVAL_CPU, POLL_INTERVAL_NETWORK, diff --git a/server/sparks/SparkMonitor.js b/server/sparks/SparkMonitor.js index b89b3be0..d4c320d8 100644 --- a/server/sparks/SparkMonitor.js +++ b/server/sparks/SparkMonitor.js @@ -6,7 +6,7 @@ import { ComfyProbe } from "../collectors/ComfyProbe.js"; import { HermesProbe } from "../collectors/HermesProbe.js"; import { TailscaleProbe } from "../collectors/TailscaleProbe.js"; import { llmDaily } from "../collectors/LlmDaily.js"; -import { sshTest, sshExec } from "../collectors/ssh.js"; +import { sshExec } from "../collectors/ssh.js"; import { POLL_INTERVAL_GPU, POLL_INTERVAL_CPU, @@ -456,23 +456,28 @@ export class SparkMonitor { try { if (this.spark.isLocal) { await this.collector.pingHost(); + if (!this._running) return; + this.online = true; + this.lastOnlineOk = Date.now(); + // Non-fatal — uptime stays at its previous value or null + try { + this._uptimeSeconds = await this._readUptime(); + } catch { + /* ignore */ + } } else { - const result = await sshTest(this.spark); + // One SSH round trip, not two. Reading /proc/uptime already proves the + // session came up, so the separate `echo ok` probe told us nothing the + // uptime read doesn't — and on a remote Spark every probe is a full + // login, which is the expensive half of this loop. + const uptimeSeconds = await this._readUptime(); // Re-check after the (up to 10s) SSH await — `stop()` may have fired // mid-flight (removeSpark / updateSpark). Bail before mutating state or // running into a stopped registry entry. if (!this._running) return; - if (!result.ok) throw new Error(result.message); - } - if (!this._running) return; - this.online = true; - this.lastOnlineOk = Date.now(); - - // Collect system uptime - try { - this._uptimeSeconds = await this._readUptime(); - } catch { - // Non-fatal — uptime stays at previous value or null + this.online = true; + this.lastOnlineOk = Date.now(); + this._uptimeSeconds = uptimeSeconds; } } catch { if (!this._running) return;