AI WIP | End-to-end sync benchmark (evidence for #3584 page-size change) - #3656
Draft
markovejnovic wants to merge 10 commits into
Draft
AI WIP | End-to-end sync benchmark (evidence for #3584 page-size change)#3656markovejnovic wants to merge 10 commits into
markovejnovic wants to merge 10 commits into
Conversation
The zero_latency_does_not_delay test asserted a hard 50ms wall-clock upper bound on a request that received Duration::ZERO latency injection. This bound flakes under loaded machines. Furthermore, tokio::time::sleep(Duration::ZERO) returns immediately by design, so this test never actually validated the is_zero() short-circuit guard in the latency middleware — removing that guard would not cause this test to fail. Delete the test, keeping the correct (but untested) guard in the middleware.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
An end-to-end benchmark for record sync throughput. A real
atuin-clientdrivesdiff→operations→sync_remoteagainst a real in-processatuin-server(temp-file SQLite) over a real loopback TCP socket. It sweeps page size (100 vs 1000) × injected RTT (0 / 20 / 100ms) over a 10,000-record corpus, in both directions.This builds the evidence for #3584 (raise the sync page size 100 → 1000, cutting HTTP round-trips 10×). It does not itself change the page size —
crates/atuin-client/src/record/sync.rsis untouched.Why the injected latency
Loopback RTT is ~zero, and RTT is exactly the cost a larger page eliminates. So the server's axum router is wrapped in middleware that sleeps a configurable RTT per request. Without it the benchmark measures SQLite and serialization instead of the thing under test.
The headline result proves the point:
(upload; download is comparable — see the results doc). At rtt=0 the two page sizes are within noise. A pure-localhost benchmark would have reported #3584's change as a rounding error.
The finding that should gate the change
axum's default 2 MB request body limit bounds page size.
handlers::v0::record::postextractsJson<Vec<Record<EncryptedData>>>and the server never overridesDefaultBodyLimit;max_record_sizeis per-record (defaults to 1 GB), so nothing else bounds the request. At ~600 bytes/record page=1000 is ~600 KB and safe — but a user with 1–2 KB commands would send 1–2 MB and could hit a 413. Before raising the page size, bound the request by bytes, not record count (or raise the server limit deliberately).Shape of the change
crates/atuin-client/benches/sync.rs— the upload/download benchmarks (divan).crates/atuin-client/benches/_util/{server,record}.rs—BenchServer(in-process server + latency middleware) and shared record generation.crates/atuin-client/tests/bench_harness.rs— real tests for the harness (a benchmark on a broken harness produces confident wrong numbers).crates/atuin-server/src/lib.rs— the one production change:make_routeris nowpubso a caller can layer middleware onto the router. Narrowest available seam (mod routeris private).crates/atuin-server/tests/make_router.rs— covers that.docs/superpowers/plans/2026-07-16-e2e-sync-benchmark*.md— the plan and the full results write-up (caveats, follow-ups, reproduction command).atuin-clientgainsatuin-server*+axumas path-only dev-dependencies (no version key, so they're stripped from the published manifest and don't affect the release publish order or what ships to users). Not a cycle —atuin-serverdoesn't depend onatuin-client.How to run
Redirecting stderr is not optional:
sync_upload/sync_downloaddraw an indicatif progress bar that indicatif suppresses only when stderr is not a TTY, which would otherwise add page-count-proportional work to exactly the comparison being made.Status
Draft / WIP. Full caveats (SQLite vs production Postgres, RTT-only latency model, synthetic payloads) and follow-ups are in the results doc.