[wchwawa]

Related Issue

Closes #532

Summary

TLDR. Each workspace RPC frame now goes out in one write, and both the client and the owner set TCP_NODELAY. On loopback, a single-RPC call drops from about 100 ms to about 10 ms. The bytes on the wire are unchanged.

Problem. The client and the owner wrote every frame as two writes: first the 4-byte length, then the body. Neither socket disabled Nagle's algorithm, so the body waited until the peer acknowledged the 4-byte segment, and the peer's delayed ACK held that acknowledgement for about 40 ms. A request and its response paid this once each.

Consequences. The native CLI, the Python SDK and the Rust SDK all use this transport and open a new connection per RPC, so every call spent about 90 ms waiting idle. Operations made of several RPCs paid it several times: publish_bytes of 64 KiB took about 540 ms.

Example. One find_workspaces call, seen from the client:

• main: writes 4 bytes, then 328 bytes. The read of the response length blocks for 52.1 ms, the read of the 516-byte body for 45.5 ms.
• this PR: writes 331 bytes once. The two reads block for 4.4 ms and 0.03 ms.

Solution.

• Client (crates/nokv-client/src/transport.rs): the request frame, length plus body, is built in one buffer and sent with one write. The buffer is built before connecting, so an oversized request still fails without touching the network. TCP_NODELAY is set right after the connection opens.
• Owner (crates/nokv-server/src/server.rs): response frames and the legacy-client rejection frame are built in one buffer and sent with one write. TCP_NODELAY is set at the start of the per-connection handler.
• The PR does both so that the result does not depend on the platform. One write alone only helps while a frame fits in one TCP segment. TCP_NODELAY alone still sends two packets per frame.

Gain. Median latency on the stage2a stack (full numbers under Claims and Evidence):

Call	main b8d59c4	this PR
find_workspaces	98.4 ms	10.2 ms
read of 64 KiB	201.9 ms	20.6 ms
publish_bytes of 64 KiB	536.0 ms	94.1 ms

Tradeoffs.

• Each outbound frame is copied once into the combined buffer. Frames are capped at 16 MiB (MAX_FRAME_BYTES), so the worst case is one extra transient copy of that size per frame in flight. Metadata frames are usually a few hundred bytes.
• A BufWriter was not used: with its default 8 KiB buffer it sends the 4-byte length on its own whenever the body is larger than the buffer, which brings the two writes back for exactly the large frames. Vectored writes would avoid the copy, but they need a hand-written loop for partial writes because Write::write_all_vectored is not stable.
• The owner sets TCP_NODELAY inside the per-connection handler, not in the accept loop. On macOS and FreeBSD, setting a socket option can fail on a connection the peer has already reset. In the accept loop that error would stop the whole owner and release its lease; in the handler it closes only that connection. On the client, a failure is reported the same way as the timeout options set right next to it, so no new kind of error appears.

Caveat for load tests (not caused by this PR). Faster RPCs raise the request rate at the owner. Every metadata write checks the shard-wide System(commit_clock) against the read version it was planned at (WriteReadVersionMismatch in crates/nokv-meta/src/workspace/engine.rs). On a mismatch the owner makes up to 8 attempts per RPC and the SDK up to 3 RPC attempts, with no backoff between them. With 16 concurrent writers on 16 different workbenches of one shard, 20 publishes each, publishes fail with metadata write read-version mismatch:

• main: 146 and 142 of 320 failed (about 45%)
• this PR: 221 and 235 of 320 failed (about 71%)

Main already fails this way. This PR removes the ~90 ms per-RPC pause that was spacing the writers out, so they collide more often. Expect these errors in concurrent load tests until the contention is fixed separately.

Not in this PR. Most of the remaining ~10 ms of a single-RPC call is the owner's accept loop: the listener is non-blocking and the loop sleeps up to 10 ms between polls. The time from connect to the handshake reply measured p50 7.0 ms and p99 10.5 ms over 1000 samples. Every RPC opens a new connection, so every RPC pays it. Reusing connections in the client would remove it, together with the per-RPC connect and handshake. That needs its own design: an idle timeout on the owner, accounting for the owner's connection permits, safe retry when a reused connection turns out to be closed, and fork safety in the Python SDK.

Scope

• This PR changes one logical boundary only: the framed TCP transport, on its client end and its owner end.
• No unrelated refactor, benchmark, metadata model, Holt layout, object-store, agent interface, or docs change is mixed in.
• The linked issue describes the user-visible problem, design decision, or maintenance task this PR resolves.
• Any breaking change is intentional and documented. There is none: the frame format, the handshake and the error types are unchanged.
• No compatibility shim, deprecated alias, or forwarding wrapper was added without a removal condition.

Change Size And Review

• I checked GitHub's additions plus deletions for this pull request: 272 lines (229 added, 43 removed), about half of them tests.
• If the total is more than 5,000 lines, one core maintainer other than the current-head pusher approved the exact current head commit. N/A: under 5,000 lines.
• This change is still small enough for a focused review; approval count is not being used to justify mixed package or lifecycle boundaries.

Code Contract (Code Changes Only)

• Not applicable; this is a docs/config-only change.
• Package boundaries follow docs/development/code_contract.md. Only nokv-client and nokv-server change, with no new dependency. Each keeps its own private frame builder because nokv-client must not depend on nokv-server.
• Shared helpers reuse the standard library or existing repository helpers. New generic helper modules are domain-neutral and tested.
• File names and file placement follow the code contract. No new files.
• New types, interfaces, structs, fields, and functions use domain-specific names.
• New errors are in the owning package's errors.rs and carry stable error kinds when crossing package boundaries. N/A: no new errors.
• New metrics/stats are owned by the package that reports or serves them. N/A: no new metrics.
• Metadata changes document durability, atomicity, revision-reference lifetime, snapshot/commit/event retention, GC epochs, and fail-closed recovery boundaries. N/A: no metadata change.
• Placement or routing changes preserve persisted RootId -> LogicalShardId affinity, one active writer per shard, owner-epoch fencing, and explicit shard-local atomicity; no split root or cross-shard transaction is implied. N/A: no placement or routing change.
• Agent-interface changes keep the transport-free facade and schemas in nokv-agent, routing and workflows in nokv-client, and the concrete backend plus native full CLI wiring in nokv. N/A: no Agent-interface change.
• User-facing integration guidance orders the native full CLI first, the direct Python SDK second, and the Rust SDK third, and does not present the deprecated nokv mcp sidecar as a supported NoKV surface. N/A: no guidance change.

Claims and Evidence

• User-facing documentation distinguishes current, experimental, and planned capabilities. N/A: no documentation change.
• Security claims do not treat a Workbench root or Agent scope as tenant authentication or authorization. N/A: no security claim.
• Performance claims state the topology, comparison boundary, cache state, run count, and raw-evidence location.
• Benchmark-only behavior does not alter product semantics. No benchmark code is added.

Evidence level and status. Product level (Python SDK, user-visible latency). Gate 8 status: NOT QUALIFIED. This is a diagnostic before/after comparison: moto stands in for the pinned RustFS because Docker was not available, it ran on one machine, and the timing harness is an ad hoc SDK script that is not committed. It compares two commits under the same recipe and is not a headline number.

Profile. Commits b8d59c4 (main at the time) and 9b1fb6e (this change on top of it), clean worktrees; each owner binary carries its commit. The branch was then rebased onto d780327 as b47e97d, which git range-diff reports as patch-identical to 9b1fb6e; the rebase was not re-measured. Rust 1.97.0, release builds of nokv and of the Python wheel from the same commit. Linux 6.18 x86_64, 4 vCPU, 15 GiB RAM, virtual disk. Stack from scripts/workbench/loopx_stage2a_stack.py up --object-store moto: one owner, one logical shard, one root, local_wal durability (owner default), etcd 3.7.1 (CI-pinned archive), moto S3 on loopback. Each variant ran on two fresh stacks, in the order main, PR, main, PR.

Sequential workload. One client, 5 warm-up calls per operation, then 200 find_workspaces(limit=100) calls, 200 read calls of one 64 KiB artifact (the SDK read has no block cache, so every call fetches from the object store), and 100 publish_bytes calls of 64 KiB of random bytes, each to a new path. Latency in ms; errors are failures after the SDK's own retries.

Call	Run	p50	p95	p99	max	ok / errors	calls/s
find_workspaces	main 1	98.4	104.1	108.0	110.7	200 / 0	10.2
find_workspaces	main 2	99.3	107.7	108.1	112.0	200 / 0	10.2
find_workspaces	PR 1	10.2	12.3	14.6	16.3	200 / 0	97.2
find_workspaces	PR 2	10.2	11.2	12.3	15.5	200 / 0	97.7
read 64 KiB	main 1	201.9	209.8	212.2	220.5	200 / 0	5.0
read 64 KiB	main 2	201.2	208.5	212.0	215.8	200 / 0	5.0
read 64 KiB	PR 1	20.6	29.2	31.9	40.6	200 / 0	45.5
read 64 KiB	PR 2	20.7	29.9	30.6	30.9	200 / 0	45.0
publish_bytes 64 KiB	main 1	536.0	564.0	568.0	580.0	100 / 0	1.9
publish_bytes 64 KiB	main 2	543.9	568.0	576.0	611.8	100 / 0	1.8
publish_bytes 64 KiB	PR 1	94.1	112.8	121.7	123.8	100 / 0	10.4
publish_bytes 64 KiB	PR 2	102.5	120.3	123.0	143.5	100 / 0	9.7

Concurrent workload. 16 threads, each with its own client and its own new workbench, started together, 20 publish_bytes of 64 KiB each. Latency of successful calls in ms. Every error was metadata write read-version mismatch.

Run	ok	errors	p50	p95	p99	max	ok/s	wall
main 1	174	146 (45.6%)	8892	14580	16924	17616	0.76	229 s
main 2	178	142 (44.4%)	8896	15292	17404	19960	0.81	220 s
PR 1	99	221 (69.1%)	8929	14472	15926	15926	0.43	229 s
PR 2	85	235 (73.4%)	9285	14087	17243	17243	0.36	236 s

Accept-loop wait (PR stack). 1000 raw handshakes with random gaps between them, timing connect to handshake reply: p50 7.0 ms, p95 10.0 ms, p99 10.5 ms, max 13.5 ms, mean 6.3 ms.

Retries are not visible through the Python SDK, so retry counts are not reported. The raw evidence is the tables above and the strace lines in the Example; the per-sample data was not kept outside the measurement machine.

Validation

All Rust and contract checks below ran on the head of this PR, b47e97d.

• cargo fmt --all -- --check: pass
• cargo clippy --workspace --all-targets -- -D warnings: pass
• cargo clippy -p nokv-server --all-targets --features restore-crash-test-support -- -D warnings: pass
• cargo test --workspace: pass, 1164 tests, 0 failed
• cargo test -p nokv-server --features restore-crash-test-support: pass, 188 tests, 0 failed
• python3 scripts/workbench/workbench_contract_test.py: pass, 8 tests
• pytest crates/nokv-python/tests against a wheel built from 9b1fb6e (before the rebase): pass, 31 tests
• git diff --check origin/main..HEAD: pass
• The new tests fail when the fix is undone. Each of these mutations, applied alone, made its test fail: client sends the length and the body as two writes; owner sends them as two writes; client without TCP_NODELAY; accepted socket without TCP_NODELAY.
• Not run (with reason): the live gates that need Docker or RustFS (local_wal_recovery_gate.py, live_workbench.py, restore_composition_gate.py, object_namespace_recovery_gate.py, fork_restore_recovery_gate.py). Docker is not available in this environment; CI runs them.

Contributor Sign-off

• Every commit in this PR includes a DCO Signed-off-by trailer.

🤖 Generated with Claude Code

https://claude.ai/code/session_012QGJnvVtHwDgtDs9poK5yN

---

Generated by Claude Code

[wchwawa]

CI status: nokv-workspace is red because of the toolchain, not because of this PR.

• Failing step: Clippy in nokv-workspace (Rust workflow). CI installs dtolnay/rust-toolchain@stable, which is now Rust 1.99. Its clippy reports needless_borrows_for_generic_args at 5 places in crates/nokv-control/src/store.rs (lines 1056, 1093, 1350, 1387, 1411), all .map_err(&conflict).

• Why this is not caused by this PR: main is red on the same check, with the same 5 errors, since d780327 (run 38831104496). This PR does not touch nokv-control.

• What else will fail after that: clippy stops at the first crate that fails, so CI has not yet checked the other crates. With Rust 1.99 on the whole workspace, the only other findings are 4 deprecation warnings, which -D warnings turns into errors: AtomicUsize::fetch_update was renamed to try_update. All 4 are in test code: crates/nokv-client/src/artifact.rs:3487 and crates/nokv-meta/src/workspace/engine.rs:4223, 4278, 4285.

• This PR's files under Rust 1.99: no clippy findings in transport.rs or server.rs. cargo fmt --check passes. cargo test -p nokv-client -p nokv-server passes: 115 and 182 tests.

• Fix: none is open yet. Proposed patch:

  • Drop the & in the 5 map_err(&conflict) calls.
  • For the 4 test sites, switch to try_update only if it is available at the declared rust-version (1.88). Otherwise keep fetch_update under a local #[allow(deprecated)].

  The patch is not folded into this PR, so that the PR stays on one boundary. Once main carries the fix, merging main into this branch makes the check meaningful again.

---

Generated by Claude Code