Skip to content

sdk/python: a handle's first call sends info and its request together - #329

Merged
CMGS merged 2 commits into
mainfrom
perf/py-pipelined-probe
Oct 5, 2026
Merged

CMGS merged 2 commits into
mainfrom
perf/py-pipelined-probe

Conversation

@CMGS

@CMGS CMGS commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

What

This finishes the Python half of #322/#324. Go's first kept call on a handle sends info and its request back to back and then reads the InfoResp. Python sent info, waited for the reply, and only then sent the request, so its first call took one round trip more than Go's.

Now every call hands its first frame to _connect:

  • On a handle's first kept dial (the probe), _connect writes info and the request together, reads the info reply under the same timeout as before, and returns the connection with the request already sent.
  • A proto-1 daemon answers info and closes without reading further, so _connect closes that connection, redials without a probe, and sends the request again.
  • Parked connections and non-probe dials go through the same send step.
  • Later frames (data chunks, data_end, stdin, stdin_close) stay where they were.
  • _lease now always takes the connection.

Changed call sites, all in sandbox.py:

  • run;
  • the fs_* RPCs: write/read/list/push/pull/find/replace;
  • _call, which covers spawn, ps, kill, stat, mkdir, remove, rename, git, sessions and lsp start;
  • _open_stream, which covers watch, pty, port and lsp requests;
  • _drain_proc and _kill_after_cut.

The second commit updates the docs/sandboxd-api.md sentence from #322: both SDKs now send frames ahead of the 101.

Numbers

Bare-metal test host, real sandboxd over plain HTTP, a byte-level TCP forwarder with a fixed 5 ms one-way delay (10 ms RTT). stat("/") p50, n=100 per run, interleaved old/new/new/old/old/new:

mode main (bed3912) this PR
first call on a fresh handle (probe) 21.86–21.92 ms 11.11–11.54 ms
keep_alive=0 (every call dials) 10.84–10.91 ms 10.86–10.89 ms

The probe path drops from two round trips to one, which matches Go (11.5 ms on the same forwarder). The dial-per-call path was already one round trip and is unchanged.

Functional check on the same node, new SDK, plain HTTP:

  • run(["sh","-c","cat | wc -c"], stdin=4 MiB, timeout=30) as the handle's first kept call → 0 4194304;
  • write_file and then read_file of 3 MiB → 3145728;
  • run with 8 MiB stdin and keep_alive=0 → 8388608;
  • exec("echo","hi") → hi;
  • a wrong token → APIError agent upgrade: {"error":"unknown sandbox"} (HTTP 404).

Behavior notes

  • A first frame sent under a run or kill deadline is bounded by the socket timeout min(client.timeout, remaining) while _connect sends it. The watchdog is armed once _connect returns, as the probe already was before this change. An exec frame is small and does not block before the socket buffers fill, so the watchdog still bounds everything a run can actually block on.
  • Hot path: plain RPCs are unchanged. A run(timeout=…) makes two extra settimeout calls.

Tests

  • test_first_frames_ride_ahead_of_the_101[proto probe]: the fake reads the upgrade headers and then two frame lines (info and the request) before it answers anything. On main it fails with ProtocolError: connection closed before the upgrade reply; with this PR it passes.
  • test_old_daemon_that_closes_after_info_gets_the_request_on_a_redial: a proto-1 fake answers info and closes, and the call succeeds on the redial. Ran 100× on macOS and 50× on Linux.
  • test_run_timeout_cuts_a_blocked_exec_send now uses a real socket (a 7 MiB exec to a port that never accepts, client timeout=30, run timeout=0.2, must raise TimeoutError within 1 s). It no longer depends on a fake conn that only waits for abort(), and it passes on main and on this branch.
  • Gates:
    • ruff format --check, ruff check, mypy and pytest (234 passed) on macOS py3.12, on the py3.9 floor and on linux/arm64 py3.12;
    • repo-root ruff;
    • sdk/openai and sdk/langchain tests;
    • e2e go test -race ., plus TestCaddyTLSCluster with Caddy 2.11.4 (the Python subtest drives the first exec through the pipelined probe over the TLS edge).

CMGS added 2 commits October 5, 2026 18:22
The proto probe sent info and waited for the reply before the caller
sent its request, so a handle's first kept call took two round trips
where the Go SDK takes one. Every call now hands its first frame to
_connect, which on the probe path writes info and the request back to
back and then reads the info reply under the same timeout as before; a
proto-1 daemon, which answers info and closes without reading further,
gets the request again on a fresh dial. A first frame sent under a run
or kill deadline is bounded by that deadline through the socket
timeout, since the watchdog is armed once _connect returns.
@CMGS
CMGS merged commit 92099e7 into main Oct 5, 2026
2 checks passed
@CMGS
CMGS deleted the perf/py-pipelined-probe branch October 5, 2026 10:37
CMGS added a commit that referenced this pull request Oct 5, 2026
…kes wire.PtyOpen) (#330)

* review: layout, one-line docs, and named func types across the post-#329 files

Applies the style rules: relayEnds moves above the Server methods it split and relay.go's free funcs below auditTee's method set; KeptNode and FakeAgent move above the tests; func types spelled twice get names (bulkMaker, chunkSink); multi-fact godocs and module docstrings drop to one line; belowFloor no longer shadows comparable; tests reuse breakStore and waitFor and drop a helper comment that restated its name.

* cut: fold single-use decode and recv layers, collect purges in the reap scan

DecodeResponse calls DecodeResponseInto with a nil scratch instead of both forwarding to a private decodeResponse, and decodeReq/decodeResp inline decodeAs. The SDK's three recv helpers become one transient read: every frame it reads is either streamed out before the next read or carries no bulk Data. reapOnce collects purge checkpoints in its locked scan, ReadFile reuses ReadFileTo, and archivesAtDeadline inlines archiveEnabledFor. Tests share one upload fake and the black_hole fixture.

* review: resolve_program's doc states where it differs from execvp

A candidate whose exec fails with ENOENT (a shebang interpreter that is gone) ends the spawn, where execvp would try the next PATH entry; the doc claimed full equivalence.

* cut: OpenPty takes wire.PtyOpen instead of a field-for-field PtyOpts copy

PtyOpts duplicated wire.PtyOpen and was copied into it on every call; the SDK already returns wire types (DirEntry, FileInfo). Callers build wire.PtyOpen directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant