perf(mountsync): linear bulk batch sizing, single read of new files, bounded concurrent uploads

Merged
KH
13d ago
+1659-75

Problem

In production, relayfile-mount --push-local-once seeds a 13,068-file (41 MB) tree in 206–218s. The server sees about 100s with no requests, then roughly 6 /fs/bulk POSTs, one at a time.

The local backlog benchmark below shows the same pattern: 6 POSTs, the first one at 61–66s, and 119 GB allocated. Almost all of that is chunkOutboxRecords re-marshalling the whole pending chunk each time it adds one record. WriteFilesBulk does the same thing again for each request. On an Apple M3 the baseline drain overruns the default 60s outbox flush budget (context deadline exceeded).

Changes

  1. Linear batch sizing. chunkOutboxRecords, WriteFilesBulk (via bulkWriteChunkBounds) and chunkPendingBulkWrites now encode each file once. They track the size as {"files":[ + items + commas + ]}, and bulkWriteRequestSizer copies the old "0 on encode error" result. The old functions are kept in the tests as reference implementations. Over a mixed corpus (empty, HTML-escaped <&>, U+2028, multibyte, base64, symlinks, draft content identities, a near-cap file and an over-cap file), with caps from 0 to 8 MiB, the tests assert that files per request and bytes per request are identical. A mutation check (an off-by-one in the comma count) fails these tests.
  2. New files are read once. When pushLocal scans, it reads small untracked regular files whole and reuses that content instead of hashing first and reading again. Limits: 1 MiB per file, 64 MiB per scan, never in low-memory mode, never for lazy-skipped files. The scan map keeps its hash-only shape, built from the same read, so the hash and the content always match. Tracked files still go through hash first, then read. Outbox writes and durability are unchanged.
  3. Bounded concurrent POSTs. flushOutboxChunks keeps up to RELAYFILE_MOUNT_BULK_UPLOAD_CONCURRENCY POSTs in flight (default 2, 1 = serial, clamped to 8). The default is 2 because Cloud's per-workspace in-flight body budget is 16 MiB, so a third 8 MiB request would only buy workspace_busy retries. flushOutboxRecordChunk is split into three steps: prepare, upload, apply.
    • Only the upload of the immutable request runs on a goroutine.
    • Prepare and apply run on the caller, which holds s.mu the whole time, in chunk order.
    • Acks come only from server responses.
    • A chunk that shares a path with an in-flight chunk waits for it.
    • After an error, no new chunks are dispatched (same as before). Uploads already in flight are still applied, so writes the server accepted get acked and are not re-sent.
    • Serial behaviour is unchanged for single chunks and for the watcher path (unlockDuringUpload).
    • 429 handling is unchanged: doJSON retries with Retry-After, and once retries run out, incrementOutboxAttempt applies backoff to that chunk only.
  4. No third tree hash. PushLocalAndFlushOnce builds the public state from pushLocal's scan instead of calling saveState(), which re-hashes every file.

Not changed

  • pushLocal flushes every 256 files, and each of those flushes is a single chunk of about 768 KB. Concurrency only helps when one flush drains several chunks: the outbox backlog drain used by --push-local-once and --flush-outbox-once, and the drain during a --notify-flush reconcile. Overlapping pushLocal's own batches would mean applying batch results later while pushLocal keeps mutating state. That is a bigger semantic change, so I left it as a possible follow-up.
  • Each record still causes two outbox file writes (pending, then acked). ensureOutboxDirs also re-reads capabilities.json on every ack. Both are now the largest local costs, and I didn't touch them because they are part of how durability works.

Benchmarks (Apple M3, real httpapi handler + relayfile.Store, 13,000 files × 3 KiB, -benchtime 1x)

Benchmarkmainthis PR
FlushOutboxOnceLargeBacklog wall109.1s / 112.7s11.0–12.0s
— first /fs/bulk after60.8s / 66.2s0.6–0.8s
— allocated118.9–120.1 GB1.88 GB
— POSTs / max in flight6 / 16 / 2
PushLocalAndFlushOnceLargeTree wall27.6s / 28.3s / 31.1s14.3s / 14.5s / 17.7s
— allocated15.8 GB2.05 GB
— allocs11.8M7.65M

The backlog benchmark sets outboxFlushTimeout = 1h so that main can finish; with the default 60s budget main fails.

Commands

GOMODCACHE=/tmp/relayfile-gomod go test ./internal/mountsync -run '^$' -bench 'LargeTree|LargeBacklog' -benchtime 1x -count 2 -benchmem   # on main (benchmark file copied in) and on this branch
go vet ./...                                   # clean
gofmt -l internal/ cmd/                        # only internal/writeback/dead-letter.go, which is unformatted on main too
go test ./... -count=1                         # all packages ok (mountsync 95s)
go test ./internal/mountsync -race -count=1 -run 'FlushOutboxChunks|PushLocal|MatchesLegacy|Sizer|ReadsNewFileOnce|Capture'   # ok

New tests:

  • TestChunkOutboxRecordsMatchesLegacyBoundaries, TestBulkWriteChunkBoundsMatchesLegacyBoundaries, TestChunkPendingBulkWritesMatchesLegacyBoundaries, TestBulkWriteRequestSizerMatchesMarshal
  • TestFlushOutboxChunksBoundsInFlightAndAcksEachRecordOnce: exactly 3 in flight, every path uploaded and acked once.
  • TestFlushOutboxChunksWorkspaceBusyKeepsAccountingExact: a 429 workspace_busy chunk stays pending with backoff, the in-flight chunk next to it is acked, later chunks are not dispatched, and recovery uploads each path exactly once.
  • TestFlushOutboxChunksSerializesSamePathAcrossChunks
  • TestFlushOutboxChunksSerialWhenConcurrencyIsOne
  • TestPushLocalAndFlushOnceReadsNewFileOnce: before, a new file was read 3 times; now once. An oversized file is still reported as writeback-skipped.
  • TestPushLocalCapturesOnlyUntrackedFiles, TestPushLocalLowMemoryDoesNotCaptureContent

Mutation checks: removing the in-flight drain after an error fails the 429 test, and removing the same-path gate fails the same-path test.

Reviewer notes / risks

  • Receipt timing against servers that return opIds. The in-process Go store gives every write an opId and dispatches it asynchronously. With 2 POSTs in flight, the receipt GET sometimes runs before the store has marked a few ops as done (14–25 of 13k in some runs). FlushOutboxOnce then returns outbox pending remains. Those records keep their opId and have zero failed attempts, and polling again 2–5s later settles all of them with no re-upload. The same race was possible with serial uploads, just less likely. Cloud's bulkWrite results carry no opId (they are acked the legacy way, with no GETs), so production should not see this. Self-hosted Go servers may hit it more often; setting RELAYFILE_MOUNT_BULK_UPLOAD_CONCURRENCY=1 restores the old timing. The benchmark tolerates only this one outcome (settleLaggingReceipts).
  • With the Go store, receipt settlement does one serial GET per file (about 13k GETs). This is not a factor on Cloud.
  • The public state written at the end of PushLocalAndFlushOnce now reflects the pushLocal scan, not a scan taken after the flush. This only affects observability.
  • Helpers newMountsyncAPIHandler, mustMountsyncTestJWT and ensureMountsyncJWTVerifier now take testing.TB so the benchmarks can use them.

No version bump or release.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


[!NOTE]
Medium Risk
Concurrent outbox flushing and reused push scans change timing and observability while preserving ack ordering in tests; mis-handling could duplicate uploads or miss errors on large drains.

Overview
Speeds up mount writeback for large --push-local-once / outbox drains by cutting redundant JSON sizing work, overlapping /fs/bulk uploads, and avoiding extra local file reads.

Bulk batch sizing encodes each file once and tracks request size incrementally (bulkWriteRequestSizer, bulkWriteChunkBounds) in chunkOutboxRecords, chunkPendingBulkWrites, and HTTPClient.WriteFilesBulk, replacing repeated full-chunk marshaling that was O(n²) on big backlogs.

Concurrent outbox flush adds flushOutboxChunks with a configurable cap via RELAYFILE_MOUNT_BULK_UPLOAD_CONCURRENCY (default 2, 1 = serial, max 8). Prepare/apply stay ordered under s.mu; only the POST runs on workers, with same-path serialization and stop-on-first-error semantics matching the old serial loop. Chunk flush is split into prepareOutboxChunkUpload / applyOutboxChunkResponse.

Local push captures small untracked files during scanLocalFilesCapturing (with fresh() revalidation) so new files are read once; PushLocalAndFlushOnce reuses that scan for public state instead of a third tree hash, dropping entries removed during flush.

Docs add the new env var; tests/benchmarks assert legacy-identical chunk boundaries and concurrency/error behavior.

Reviewed by Cursor Bugbot for commit 8d123436af28ca0db748c8e1801ee4bccea4555a. Bugbot is set up for automated code reviews on this repo. Configure here.


Agent Relay sessions

  • claude session a26b88d9 · opened via gh pr create · last active 2026-10-10
  • claude session 5d87b716 · contributor · ran gh pr commands · last active 2026-10-10