TC-410: performance optimization

[samgbafa]

TC-410: https://linear.app/tinycloud-labs/issue/TC-410/perfnode-stop-cloning-authorization-into-object-metadata

Summary

In tinycloud-node-server/src/auth_guards.rs: added a new InvokeHeaders<'r> FromRequest guard replacing ObjectHeaders' FromRequest impl (ObjectHeaders itself is retained solely as the stored-metadata Responder). InvokeHeaders borrows the last Rocket-parsed value (via headers.get(name).last(), zero-alloc) for accept, content-type, if-match, if-none-match, x-tinycloud-expected-version, x-tinycloud-max-response-bytes, x-tinycloud-limit, and x-tinycloud-cursor, and owns only a Metadata map built from the existing storable allowlist (content-type/encoding/language/disposition + x-tinycloud-meta-*). Fixed is_storable_object_header to be allocation-free and ASCII-case-insensitive (previously called to_ascii_lowercase(), allocating a String on every check) using eq_ignore_ascii_case and a byte-slice prefix compare. Added connection_nominates, which scans Connection header values via split(',') (no token-set allocation) so any storable header nominated by a comma-separated Connection token is excluded from persisted metadata — Authorization, Cookie, and all hop-by-hop headers were already excluded by the allowlist regardless. In tinycloud-node-server/src/routes/mod.rs: switched both #[cfg(feature="duckdb")]/#[cfg(not(...))] invoke() variants and invoke_impl to take InvokeHeaders<'_> instead of ObjectHeaders; updated kv_invoke_options, kv_invoke_options_for_capabilities[_with_cursor], is_multipart, and build_batch_kv_inputs to read the borrowed fields directly instead of mutating/taking from an owned Metadata map; removed the now-unused metadata_header/take_metadata_header helpers; changed parse_positive_u64_header to accept Option<&str> directly; replaced the DuckDB Arrow-Accept check to read headers.accept directly; the non-multipart KV put path now moves headers.metadata once into filter_stored_object_metadata (defense-in-depth filter retained unchanged). Updated three unit tests (bounded_kv_headers_are_positive, kv_create_and_replace_headers_build_exact_key_preconditions, kv_condition_headers_reject_ambiguous_or_batch_mutations) to construct InvokeHeaders fixtures instead of ObjectHeaders. No changes to authorization, revocation, replay-cache, timestamp validation, database schema, CORS, signed-URL, SDK, or deployment/infrastructure code.

Acceptance Criteria

1. ObjectHeaders remains the stored-metadata responder; a lifetime-bound InvokeHeaders guard becomes the /invoke request guard in both feature variants and invoke_impl.
2. InvokeHeaders borrows the last Rocket value for Accept, Content-Type, If-Match, If-None-Match, x-tinycloud-expected-version, x-tinycloud-max-response-bytes, x-tinycloud-limit, and x-tinycloud-cursor. It owns only content-type, content-encoding, content-language, content-disposition, and x-tinycloud-meta-* metadata.
3. Header-name checks are allocation-free and ASCII-case-insensitive. Rocket 0.5.1 behavior is preserved: differently cased names form one logical header, the first inserted casing is retained, values remain FIFO, and the current BTreeMap result selects the last value.
4. Authorization, Cookie, Proxy-Authorization, Proxy-Authenticate, Proxy-Connection, Connection, Content-Length, Keep-Alive, TE, Trailer, Transfer-Encoding, Upgrade, and arbitrary headers are never materialized as object metadata. Authorization’s value is read only by AuthHeaderGetter.
5. All Connection values are scanned without allocating a token set. Any otherwise storable metadata header nominated by a comma-separated Connection token is not persisted. Borrowed control parsing remains compatible, including Content-Type handling.
6. Existing handler parsing and exact status/message behavior remain unchanged, including the explicit 400 rejection for x-tinycloud-expected-version, preconditions, limits, cursors, multipart boundaries, and DuckDB Accept negotiation.
7. Non-multipart KV put moves the guard’s owned metadata once and retains filter_stored_object_metadata as defense-in-depth. Multipart part metadata continues through the same storable allowlist. Responder-side replay filtering remains separate and unchanged.
8. No authorization, revocation, durable replay-cache, timestamp, database-schema, CORS, signed-URL, SDK, or protocol changes are included. Production TLS, ingress normalization, deployment topology, rollout, and observability are follow-ups, not merge gates.

Test Plan

Add Rocket-backed characterization tests before refactoring that establish logical case folding, first-name casing, FIFO duplicates, last-value selection, and multiple Connection-line behavior. Add InvokeHeaders unit tests for mixed case and repeated values; approved metadata and every borrowed control must preserve current selection, while hostile headers are absent.

Add deterministic test-only accounting at the selection seam: selected-entry count and owned name/value bytes. Compare otherwise identical small-Authorization and representative 8–16 KiB/depth-4 requests. Copied Authorization bytes must be exactly zero, and both counts must be identical; adding approved metadata must change counts only by its attributable bytes.

Add an end-to-end /invoke KV put followed by metadata/read coverage. Approved metadata must persist and replay; Authorization, Cookie, fixed hop-by-hop fields, Proxy-Connection, and a Connection-nominated x-tinycloud-meta-* field must not persist or replay. Retain defense-in-depth tests for legacy metadata and multipart part filtering.

Run focused precondition, limit, cursor, multipart, Arrow-Accept, response-streaming, durable-replay, revocation, and w1_native_contract tests. Final gates: cargo fmt --all -- --check; cargo clippy -p tinycloud-node --all-targets -- -D warnings; cargo test -p tinycloud-node; and DuckDB-feature coverage sufficient to compile and exercise borrowed Accept behavior.

Benchmark Plan

Use .context/benchmarks/profiles/TC-410.json unchanged with pinned driver revision 9d4866f, its serverBuildCommand and CARGO_TARGET_DIR, 5 warmups, 50 samples, and 5 paired rounds. Both origin/main baseline and candidate must be release builds. Record exact SHAs and profile hash, use the same quiet host/configuration, and interleave baseline/candidate order across rounds; debug/release mixing or unpaired runs invalidates the result.

Targets are sdk.kv.get, sdk.kv.list, sdk.kv.put, sdk.sql.execute, and sdk.sql.query, especially their .http.headers.post.invoke phases; corresponding .http.total.post.invoke and operation totals are secondary target evidence. Non-targets are every other scenario or phase emitted by the profile and remain regression sentinels.

For every pair, report raw samples plus mean, p95, and p99 deltas. A mean regression counts only when it exceeds both 5% and 0.15 ms for either target or non-target metrics; p95 requires both 7.5% and 0.25 ms; p99 requires both 15% and 0.5 ms. A merge-blocking regression requires the same adverse threshold breach in at least 4 rounds.

Before merge, attach release-build provenance, raw outputs, paired-round comparison, and deterministic copy-accounting results. If latency is below timer noise, merge is acceptable only with zero redundant Authorization copying and no consistent regression; do not claim a speedup. If the profile lacks separate small/depth-4 latency cases, document that without altering it and use deterministic accounting as the size-scaling proof.

[samgbafa]

Smithers evidence run for TC-410

Benchmark verdict: regressed

The TC-410 commit (7b7585e, only auth_guards.rs +74 and routes/mod.rs 151 lines) strictly removes per-request work: the old ObjectHeaders guard cloned every header (including Authorization) into an owned BTreeMap; the new InvokeHeaders<'r> guard borrows control headers and materializes only the storable allowlist. Nothing in this diff plausibly adds 11ms to kv.get tails — the guard runs identically for put/list/sql scenarios, whose p99s improved. Three pieces of evidence point to measurement artifact: (1) the baseline run itself carries wandering ~20-26ms p99 outliers in kv.put and sql.query (5x their own p95s) that vanish in the candidate run while an equivalent ~17ms outlier appears in kv.get — one slow window per run landing in different scenarios; (2) the profile uses rounds=5/samples=50, so p99 is effectively the single slowest request of 50 — a one-request GC pause, SQLite checkpoint, or dropped keep-alive per round flips the gate; (3) the non-target crypto.decrypt p99 (+227%) cannot be affected by server header handling, confirming run-level tail instability. Additionally, the branch is stacked: main..HEAD contains b0bf4b1 (TC-409, #200) and a62a92f (TC-313, #201), and local main (593c4d8) is 2 commits behind origin/main. If the benchmark baseline was built from stale local main, the measured delta conflates TC-409's admission/replay-persistence changes (admission.rs, db.rs, invocation_replay.rs — plausible sources of read-path tail stalls via DB write contention) with TC-410's header work.

Next actions:

• Do not adjust noise thresholds to force a pass; the gate behaved as configured. Instead fix the measurement: rebuild the baseline from the branch parent b0bf4b1 (or origin/main including TC-409: performance optimization #200/TC-313: fix probe empty-env overrides + self-provision space #201) and rerun the TC-410 profile with samples>=200 and interleaved rounds on a quiesced machine.
• If the rerun verdict is pass (expected, given only kv.get p99 tripped and the diff removes work), record the corrected baseline provenance and rerun artifacts in the PR and Linear comment, then proceed to merge approval — review, checks, and CI are already green.
• If kv.get p99 still regresses >=15% with >=0.5ms effect across 4+ rounds against the corrected baseline, bisect between b0bf4b1 and 7b7585e to rule out TC-409's admission/replay persistence as the tail source, and capture a flamegraph or tracing spans on the outlier requests before touching TC-410 code.
• Post a Linear comment on TC-410 summarizing: merge blocked only by benchmark verdict; three kv.get p99 entries flagged; evidence of tail-noise migration (baseline kv.put p99 26.8ms→6.4ms, sql.query 25.1ms→6.9ms, non-target crypto p99 +227%); stale-baseline risk from the stacked TC-409/TC-313 commits; and the rerun plan above.
• Attach the allocation/byte-copy evidence required by the issue's acceptance checklist to the PR before the next merge attempt so the ticket can be closed cleanly.

[samgbafa]

Benchmark provenance correction: the baseline was not stale. All five baseline summaries identify server revision b0bf4b10d14f485499e4104a9a7e23212cf71ce8, which is exactly candidate 7b7585e8977399e0902a4c7598f48d9a1808cc14's parent and includes TC-409/#200 plus TC-313/#201. All five candidate summaries identify 7b7585e…; both arms identify pinned client 9d4866fbb8415373737522698b388c950d70c1ee, 5 warmups, and 50 samples.

Therefore Fable's stale-baseline concern is ruled out, but the regressed gate still stands: KV-get p99, header p99, and total p99 each increased by ~11 ms and crossed the 4/5 consistency threshold. The PR remains open and unmerged. The useful Fable follow-ups are to raise samples to >=200, interleave A/B rounds on a quiesced machine, instrument server-vs-network outliers if reproduced, hoist Connection-token parsing if warranted, and attach the issue's requested small/depth-4 allocation evidence.

Artifacts are local under .context/benchmarks/TC-410/runs/{b0bf4b10d14f485499e4104a9a7e23212cf71ce8,7b7585e8977399e0902a4c7598f48d9a1808cc14}.