Add opt-in flow control: --flow-control {backpressure,discard} (#631) - #730
Add opt-in flow control: --flow-control {backpressure,discard} (#631)#730bmaurer wants to merge 3 commits into
Conversation
|
Cool! I'm glad you had a chance to look into this! Question: the way I typically use ET is I kick off a training job and then close my laptop. Will this stall the training process? Do we need logic that disables flow control when there's no client connected? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #730 +/- ##
==========================================
- Coverage 87.01% 86.93% -0.09%
==========================================
Files 72 74 +2
Lines 5639 5917 +278
Branches 531 572 +41
==========================================
+ Hits 4907 5144 +237
- Misses 731 773 +42
+ Partials 1 0 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Looking at TerminalServer.cpp, I think the flow control logic works like this:
Does that sound right? At the end of the day, the behavior here is that eventually a write() to the console will buffer. that may or may not stall the app depending on implementation. Logic dictates that either (1) we need to have an infinite buffer or (2) we need to backpressure. The correct approach to this is to use tmux -CC, which i believe has it's own flow control https://github.com/tmux/tmux/wiki/Control-Mode. Any other approach will either buffer infinitely or stall. Maybe we should do a smaller buffer when you are connected to the server but a larger buffer when not. |
|
btw, is there a good way to do an end to end test of this (like testing out et against a program that does log spew, have the client disconnect then reconnect) I struggled to vibe code that part |
I've been slammed with interviews lately but once I have some time, my plan (feel free to do this yourself) is to start a pytorch training job inside tmux -CC inside et, close the connection, then come back an hour later and make sure the training kept running. |
|
By "close the connection" I mean turn off wifi |
9f4ac9a to
5240cc2
Compare
bmaurer
changed the title
|
I've reworked this PR substantially to address the concern raised here and in #631 — that always-on backpressure would cause the "process blocks while disconnected" problem. The default is now byte-for-byte the current behavior. Flow control only activates when a client explicitly passes The two opt-in modes map to the two camps in the issue thread:
The other significant finding while measuring this: application-level buffering alone barely helps, because the real queue lives in the kernel's autotuned TCP send buffer (up to 20MB on my test host) where nothing can be gated or dropped. That's why opted-in sessions set The branch is now a 3-commit stack (standalone EBADF busy-loop fix → feature → E2E harness); each commit builds and passes the test suite independently. |
5240cc2 to
d6ca0cb
Compare
|
Rebased onto current master (v7.0.0) and re-validated everything. The rebase was interesting because v7.0.0 independently landed adjacent work: disconnect buffering (#731, #762 — the connection now buffers up to 64MB while disconnected so processes don't freeze immediately), the PTY-input deadlock fix (#765), console output coalescing (#742), and port-forward fds in select() (#736). This PR now composes with all of that: the Re-measured on the v7.0.0 base, same harness (100KB/s link, 2.7MB/s producer):
The v7 disconnect work doesn't change the connected-saturated-link behavior (writes still land in the autotuned multi-MB kernel send buffer), so #631 still reproduces on the baseline — the kernel-queue tuning here is what bounds it. Full test suite passes (130 tests) and the disconnect scenarios were re-run: in discard mode the producer runs at full speed through a 60s outage and the display recovers to ~1.1s of lag within one sample of reconnecting. |
waitOnSocketData treated EBADF/EINVAL from select() like EINTR and returned false. Callers (SocketHandler::readAll, RawSocketUtils::readAll) treat false as "retry", but a closed fd never becomes readable again, so an fd closed by another thread spun at 100% CPU. Throw instead, so retry loops treat it as socket death like any other read failure. The etterminal and jumphost init loops read packets outside any try/catch; give them one so a router that dies during init produces a logged fatal instead of an uncaught exception.
…isterTea#631) When a process produces output faster than the network can deliver it, the excess queues in kernel buffers - mostly the server's TCP send buffer, which autotunes to many MB. On a saturated link the display falls steadily behind real time and Ctrl-C appears dead: the interrupt is delivered, but the prompt has to wait behind the entire queue. On a 100KB/s test link, Ctrl-C took 56-116s to take effect. This adds a per-session flow control mode, chosen by the client: - none (default): the existing behavior, unchanged. The mode field is not even serialized when unset, so new binaries interoperate with old ones exactly as before, and users who don't opt in see zero change. - backpressure: terminal output is staged in a small (64KB) buffer that is drained only when select() reports the socket writable; when it fills, the server stops reading the PTY and the remote process pauses, like plain ssh. Lossless - safe for consumers that need every byte, such as tmux -CC. - discard: same buffer, but when it fills the oldest chunk is dropped. The remote process never stalls - even while the client is disconnected - and the display stays near real time. Output that exceeded the link rate is missing from scrollback. Application-level buffering only helps if the backlog actually waits in the application buffer, so opted-in sessions also shrink the kernel queues via SocketHandler::minimizeKernelBuffering (a no-op by default): - TCP_NOTSENT_LOWAT=32KB on the client connection: select() reports writable only when <32KB is unsent, keeping the backlog in the WriteBuffer where it can be gated or dropped. In-flight (sent-but-unacked) data is not limited, so throughput on high-BDP links is unaffected. It is reapplied every loop iteration because the connection gets a new socket on reconnect and fd numbers are reused. - SO_SNDBUF=64KB on the etterminal->etserver unix socket (pure queue, no in-flight component, so clamping costs nothing locally). etterminal learns the mode via a new TermInit field. In the jumphost path, discard only drops TERMINAL_BUFFER packets; control packets (port forwarding, responses, keepalives) are always delivered, and reads pause when the queue is dominated by them so it stays bounded. The client suppresses its missed-keepalive disconnect while it is intentionally not reading (console buffer full), since the keepalive echo may be sitting unread in the socket. Measured on a 100KB/s throttled link with a 2.7MB/s producer: display lag @30s producer Ctrl-C -> prompt none (unchanged) ~31s, growing throttled 56-116s backpressure ~2.5s, bounded throttled 2-3s discard ~1.2s, bounded full speed 2.1s In discard mode the producer also runs at full speed through a 60s network outage, and on reconnect the display recovers to ~1.2s of lag instead of replaying the backlog.
A userspace throttle proxy (100KB/s, with a clamped receive buffer so it models a real slow link instead of absorbing megabytes into its own autotuned rcvbuf) sits between etserver and the et client, with a timestamp-printing workload behind a PTY driven from tmux. - do_scenario.sh: per-mode scenario (trunk/none/backpressure/discard) measuring display lag, producer lag (is the process stalled?), throughput, and Ctrl-C-to-prompt latency, with an optional 60s disconnect simulation. Emits JSONL metrics. Refuses to run on a dirty tree because its cleanup restores source dirs from HEAD. - throughput_test.sh: bulk throughput on a fast link (no throttle). - REPORT.md: methodology and measured results. To run the components standalone, et gains --idpasskey to skip the SSH handshake and connect directly to a manually started etterminal, and etterminal's --idpasskey handling is fixed to actually use the passed value (it was assigned to a shadowing local and dropped).
d6ca0cb to
201fdb4
Compare
2 participants
Problem (#631)
When a process produces output faster than the network can deliver it, the excess queues in kernel buffers — mostly the server's TCP send buffer, which autotunes to many MB. On a saturated link the display falls steadily behind real time and Ctrl-C appears dead: the interrupt is delivered, but the prompt has to wait behind the entire queue. On a 100KB/s test link, Ctrl-C took 56–116 seconds to take effect.
Design: strictly opt-in flow control
This revision makes flow control a per-session mode chosen by the client, with the default being no change at all:
--flow-control none(default) — the existing code path, unchanged. The proto field isn't even serialized when unset, so new/old clients and servers interoperate exactly as before. Users who don't opt in are unaffected.--flow-control backpressure— terminal output is staged in a small (64KB) buffer drained only whenselect()reports the socket writable; when it fills, the server stops reading the PTY and the remote process pauses, like plain ssh. Lossless — safe for consumers that need every byte, e.g.tmux -CC.--flow-control discard— same buffer, but when full the oldest chunk is dropped. The remote process never stalls — even while the client is disconnected — and the display stays near real time. Output that exceeded the link rate is missing from scrollback.Kernel buffer tuning (opt-in sessions only)
Application-level buffering only helps if the backlog actually waits in the application buffer, so opted-in sessions also shrink the kernel queues (
SocketHandler::minimizeKernelBuffering, a no-op by default):TCP_NOTSENT_LOWAT=32KBon the client connection:select()reports writable only when <32KB is unsent, keeping the backlog in the WriteBuffer where it can be gated or dropped. In-flight (sent-but-unacked) data is not limited, so throughput on high-BDP links is unaffected. Reapplied each loop iteration because reconnects replace the socket (and fd numbers get reused).SO_SNDBUF=64KBon the etterminal→etserver unix socket (a pure queue with no in-flight component, so clamping costs nothing locally). etterminal learns the mode via a newTermInitfield.In the jumphost path, discard only drops
TERMINAL_BUFFERpackets; control packets (port forwarding, responses, keepalives) are always delivered, and reads pause when the queue is dominated by them so it stays bounded.Measured results (100KB/s throttled link, 2.7MB/s producer)
none(unchanged)backpressurediscardWith
discard, the producer also runs at full speed through a 60s network outage, and on reconnect the display recovers to ~1.2s of lag instead of replaying the backlog.Commits
select()was treated like EINTR, soreadAllretry loops spun at 100% CPU on a concurrently-closed fd.WriteBufferTest) and an integration test covering both opt-in modes end-to-end (ServerFlowControlDataTransferTest).test/e2e/REPORT.md.Fixes #631