Keep idle SSH tunnel sessions alive on dedicated clusters - #6382
Keep idle SSH tunnel sessions alive on dedicated clusters#6382anton-107 wants to merge 1 commit into
Conversation
The websocket keepalive ping never reaches a dedicated cluster: the driver proxy answers control frames itself, so the tunnel leg past it stays idle and is reaped after ~8m20s. Generate real SSH payload instead, from both ends of the tunnel — ServerAliveInterval on the ssh client the CLI spawns and in the host config it writes, and ClientAliveInterval in the sshd config the tunnel's server writes. Co-authored-by: Isaac <no-reply@databricks.com>
Integration test reportCommit: 4fe97ea
Top 3 slowest tests (at least 2 minutes):
|
rugpanov
left a comment
rugpanov
left a comment
There was a problem hiding this comment.
Reviewed end-to-end — approach, the three keepalive sites, and the safety margins. Nothing blocking; good to ship for today's release.
The evidence is convincing: the websocket ping is a control frame the driver proxy terminates itself (proxy/client.go:111 — "driver proxy does not return pongs"), so it can't keep the leg past the proxy warm, and the e2e table shows exactly that (30 pings / 0 pongs, death at +8m20s). The SSH-level keepalive reply is real payload, which is the fix. Coverage hits all three ssh-invocation sites (client.go:823, sshconfig.go:224, sshd.go:110), and the 3 x interval = ~90s teardown clears the up-to-30s handover pause with margin.
A few optional polish items below — all non-blocking, fine as a quick follow-up:
ServerAliveIntervalSecondsandclientAliveIntervalSecondsare the same value in two packages (neither imports the other) and can silently drift — either share one constant or reword the "the same keepalive" wording so the independence is intentional. Your call on the layering.- The three new test names are much more prose-y than neighbors like
TestBuildRemoteShellArgs— purely cosmetic.
| configDirName = ".databricks/ssh-tunnel-configs" | ||
| ) | ||
|
|
||
| // ServerAliveIntervalSeconds is how often the ssh client asks the SSH server to confirm it is |
There was a problem hiding this comment.
nit (optional): this doc-comment runs 13 lines / 2 paragraphs where the rest of the package is 1-2 (configDirName, containsLine). The non-obvious core is worth keeping — SSH keepalive puts payload past the proxy where the websocket ping can't, the ~8-min reap window, DECO-28186, and the CountMax/handover interaction. The customer-anecdote narrative ("verified over ~2 hours idle", "measured to hold...") is the trimmable part; could be roughly half the length without losing the "why".
| "github.com/databricks/databricks-sdk-go" | ||
| ) | ||
|
|
||
| // clientAliveIntervalSeconds is how often sshd asks the client to confirm it is still there. It is |
There was a problem hiding this comment.
nit (optional): this re-explains the payload/reap mechanism that sshconfig.ServerAliveIntervalSeconds already documents and cross-references — it could defer to it ("see its doc for the why") and keep only the distinct part (covers hand-written ProxyCommand / IDE-supplied clients the CLI doesn't configure).
Also: this = 30 and ServerAliveIntervalSeconds = 30 are the same value, decoupled, and can silently drift. Either share one constant or reword "the same mechanism" to make the independence intentional. Non-blocking — flagging as a conscious choice.
| func TestBuildSSHArgsAsksTheServerToConfirmItIsStillThere(t *testing.T) { | ||
| args := buildSSHArgs("user", "/key", "proxy command", "myhost", "", ClientOptions{}) | ||
|
|
||
| // An idle session sends no payload of its own, and payload is the only traffic that keeps the |
There was a problem hiding this comment.
nit: the "idle session sends no payload... reaped (DECO-28186)" opening here is repeated across the three new tests (verbatim in the sshconfig test, paraphrased in the sshd one) and now also lives on the constant's doc. Could keep just the assertion-specific part — why the option must precede the destination host.
| func TestSSHDConfigAsksTheClientToConfirmItIsStillThere(t *testing.T) { | ||
| config := sshdConfigContent("/keys/server-private-key", "/keys/authorized_keys", `SetEnv FOO="bar"`) | ||
|
|
||
| // sshd sends nothing on an idle session unless ClientAliveInterval is set, and payload is the |
There was a problem hiding this comment.
nit: same duplicated DECO-28186 opening as the other two new tests — the "half the CLI never configures" note is the part worth keeping. Separately, assert.Greater(t, 3*clientAliveIntervalSeconds, 30) hardcodes the handover pause; proxyHandoverInitTimeout is unexported in the proxy package so it can't be referenced directly — a short comment tying the 30 to its current value would keep the bound from going stale silently.
| func TestGenerateHostConfigAsksTheServerToConfirmItIsStillThere(t *testing.T) { | ||
| config := GenerateHostConfig("myhost", "root", "/keys/myhost", "databricks ssh connect --proxy") | ||
|
|
||
| // An idle session sends no payload of its own, and payload is the only traffic that keeps the |
There was a problem hiding this comment.
nit: same as the sshd test — the leading DECO-28186 sentence duplicates the constant's doc; keeping just "ssh setup and --ide reach ssh through this block and nothing else" is enough. Same hardcoded-30 note applies to the bound below.
| args := buildSSHArgs("user", "/key", "proxy command", "myhost", "", ClientOptions{}) | ||
|
|
||
| // An idle session sends no payload of its own, and payload is the only traffic that keeps the | ||
| // tunnel leg past the driver proxy from being reaped (DECO-28186). ssh stops parsing options |
There was a problem hiding this comment.
| // tunnel leg past the driver proxy from being reaped (DECO-28186). ssh stops parsing options | |
| // tunnel leg past the driver proxy from being reaped. ssh stops parsing options |
| // the same mechanism as the client's own ServerAliveInterval (sshconfig.ServerAliveIntervalSeconds) | ||
| // driven from the other end of the tunnel: the reply is a real SSH packet, so an idle session still | ||
| // puts payload bytes on every hop, which is the only kind of traffic that keeps the leg past the | ||
| // driver proxy from being reaped (see DECO-28186). Configuring it here covers clients the CLI does |
There was a problem hiding this comment.
| // driver proxy from being reaped (see DECO-28186). Configuring it here covers clients the CLI does | |
| // driver proxy from being reaped. Configuring it here covers clients the CLI does |
| config := sshdConfigContent("/keys/server-private-key", "/keys/authorized_keys", `SetEnv FOO="bar"`) | ||
|
|
||
| // sshd sends nothing on an idle session unless ClientAliveInterval is set, and payload is the | ||
| // only traffic that keeps the tunnel leg past the driver proxy from being reaped (DECO-28186). |
There was a problem hiding this comment.
| // only traffic that keeps the tunnel leg past the driver proxy from being reaped (DECO-28186). | |
| // only traffic that keeps the tunnel leg past the driver proxy from being reaped. |
| // still there. The reply is a real SSH packet, so the keepalive puts payload bytes on every hop | ||
| // of the tunnel — and payload is what an idle session needs. The driver proxy terminates | ||
| // websocket control frames itself, so the proxy's own websocket ping never becomes payload on | ||
| // the leg past it, and that leg is reaped after ~8 minutes without any (see DECO-28186). Setting |
There was a problem hiding this comment.
| // the leg past it, and that leg is reaped after ~8 minutes without any (see DECO-28186). Setting | |
| // the leg past it, and that leg is reaped after ~8 minutes without any. Setting |
| config := GenerateHostConfig("myhost", "root", "/keys/myhost", "databricks ssh connect --proxy") | ||
|
|
||
| // An idle session sends no payload of its own, and payload is the only traffic that keeps the | ||
| // tunnel leg past the driver proxy from being reaped (DECO-28186). `ssh setup` and `--ide` |
There was a problem hiding this comment.
| // tunnel leg past the driver proxy from being reaped (DECO-28186). `ssh setup` and `--ide` | |
| // tunnel leg past the driver proxy from being reaped. `ssh setup` and `--ide` |
4 participants
Changes
#6358 fixes idle-session drops on serverless and does nothing for a dedicated cluster. Its websocket ping is a control frame: the driver proxy terminates it itself, so it never becomes payload on the leg past it, and that leg is reaped after ~8m20s of carrying nothing. Zero pongs came back in any run on either compute type — the frame does not traverse. Serverless sits behind a different reaper that the ping does reset, which is why it fixes serverless and only serverless.
The fix is to generate traffic the tunnel forwards end to end. An SSH keepalive reply is a real SSH packet, so the proxy loops carry it as a websocket data frame across every hop — which is why setting
ServerAliveIntervalby hand is a total workaround. The CLI now asks for it itself, from both ends:ServerAliveInterval 30inbuildSSHArgs(theconnectpath) andGenerateHostConfig(thesetupand--idepaths). Needs nothing from the compute, so it also fixes a cluster already running a server binary from an older CLI.ClientAliveInterval 30in the sshd config the tunnel's server writes. Covers clients the CLI never configures — a hand-writtenProxyCommandblock, or an IDE supplying its own ssh options.Either half alone is sufficient (measured below). Both are in because they close different gaps and neither subsumes the other. The websocket ping stays: it is what fixes serverless, and it keeps the client↔control-plane hop warm regardless of the user's ssh config.
One behaviour change, deliberately. These options bring in
ServerAliveCountMax/ClientAliveCountMax(OpenSSH default 3), so a tunnel that stops responding is now torn down after ~90s with ssh's own "server not responding" message instead of hanging. That is the trade we want — today a reaped idle session is a silent black hole, which is the original report — and 90s is well clear of the longest legitimate pause on a healthy tunnel, the up to 30s a handover can hold the sending loop (proxyHandoverInitTimeout).sshdConfigContentisprepareSSHDConfig's config string moved verbatim into a function plus the one new line, so the content is testable without mocking three secret reads. #6358's changelog fragment is updated rather than joined by a second, contradicting one.An out-of-band keepalive frame of our own was rejected:
runReceivingLoopwrites every binary message straight into the SSH byte stream, so it needs a framing change on both ends and only helps once the uploaded server binary is new enough.Resolves DECO-28186 for dedicated clusters; #6358 resolves it for serverless.
Why
Dedicated compute is what the README requires for remote development in an IDE, so the compute type most affected was the one #6358 left broken. Shipping only the websocket ping would close the ticket with the reported failure still in place for those users, and add a log line that makes an idle session look healthy while it is dying.
Tests
Three unit tests, one per site the directive has to appear in, each also bounding the interval against the reap window and against the handover pause. All six mutants killed: dropping the option from each of the three sites, setting either constant to
0, and pushing the interval past the reap window../task test-exp-ssh,./task lint(0 issues, all three modules),./task fmt, and the whitespace, deadcode and changelog checks are all clean; the four touched packages are green under-race.End-to-end (dogfood, dedicated clusters)
One dedicated single-node cluster per build — two tunnels on one cluster collide on port 7772 and take each other down. DBR 17.3 LTS, m5d.xlarge, SINGLE_USER; distinct pinned version per build, since
uploadReleasesskips the upload when the versioned workspace path already exists. Every session ran on a real PTY, was left completely untyped for the whole window, then sent exactly one command. Idle is measured remote-clock to remote-clock.ServerAliveIntervalClientAliveIntervaldc-basedc-fixdc-sshd-onlydc-fix-longdc-basereproduces the bug on Keep idle SSH tunnel sessions alive with a websocket ping #6358's own head, as reported: alive-looking for the whole window, killed by the first keystroke. The server lost its half at18:04:46— 8m20s after the session went quiet — withwebsocket: close 1006 (abnormal closure): unexpected EOF. The client had sent 30 pings with zero failures by then; its next two failed, after the server was already gone. Endedclose 4000: Handler crashed: ...ClosedStreamException,exit status 255.dc-sshd-onlyisolates the server half: same build, but ssh invoked by hand with-o ServerAliveInterval=0, so sshd drives the only SSH-level keepalive. A command-line-obeats any config file in OpenSSH, andClientAliveInterval 30was read back off the cluster's generatedsshd_config. The complementary control for the client half — old server binary, client-side option only — passed earlier on the same setup.dc-fix-longis past two reap windows, so the PASS is a real fix rather than a delayed drop.Full measurements and log excerpts are on DECO-28186. The test clusters have been terminated.
This PR and its description were written by Isaac.