perf(mountsync): linear bulk batch sizing, single read of new files, bounded concurrent uploads
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
- Linear batch sizing.
chunkOutboxRecords,WriteFilesBulk(viabulkWriteChunkBounds) andchunkPendingBulkWritesnow encode each file once. They track the size as{"files":[+ items + commas +]}, andbulkWriteRequestSizercopies 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. - New files are read once. When
pushLocalscans, 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. - Bounded concurrent POSTs.
flushOutboxChunkskeeps up toRELAYFILE_MOUNT_BULK_UPLOAD_CONCURRENCYPOSTs 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 buyworkspace_busyretries.flushOutboxRecordChunkis 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.muthe 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:
doJSONretries with Retry-After, and once retries run out,incrementOutboxAttemptapplies backoff to that chunk only.
- No third tree hash.
PushLocalAndFlushOncebuilds the public state frompushLocal's scan instead of callingsaveState(), which re-hashes every file.
Not changed
pushLocalflushes 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-onceand--flush-outbox-once, and the drain during a--notify-flushreconcile. OverlappingpushLocal's own batches would mean applying batch results later whilepushLocalkeeps 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).
ensureOutboxDirsalso re-readscapabilities.jsonon 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)
| Benchmark | main | this PR |
|---|---|---|
FlushOutboxOnceLargeBacklog wall | 109.1s / 112.7s | 11.0–12.0s |
— first /fs/bulk after | 60.8s / 66.2s | 0.6–0.8s |
| — allocated | 118.9–120.1 GB | 1.88 GB |
| — POSTs / max in flight | 6 / 1 | 6 / 2 |
PushLocalAndFlushOnceLargeTree wall | 27.6s / 28.3s / 31.1s | 14.3s / 14.5s / 17.7s |
| — allocated | 15.8 GB | 2.05 GB |
| — allocs | 11.8M | 7.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,TestBulkWriteRequestSizerMatchesMarshalTestFlushOutboxChunksBoundsInFlightAndAcksEachRecordOnce: exactly 3 in flight, every path uploaded and acked once.TestFlushOutboxChunksWorkspaceBusyKeepsAccountingExact: a 429workspace_busychunk 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.TestFlushOutboxChunksSerializesSamePathAcrossChunksTestFlushOutboxChunksSerialWhenConcurrencyIsOneTestPushLocalAndFlushOnceReadsNewFileOnce: before, a new file was read 3 times; now once. An oversized file is still reported aswriteback-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
opIdand 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).FlushOutboxOncethen returnsoutbox 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'sbulkWriteresults carry noopId(they are acked the legacy way, with no GETs), so production should not see this. Self-hosted Go servers may hit it more often; settingRELAYFILE_MOUNT_BULK_UPLOAD_CONCURRENCY=1restores 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
PushLocalAndFlushOncenow reflects thepushLocalscan, not a scan taken after the flush. This only affects observability. - Helpers
newMountsyncAPIHandler,mustMountsyncTestJWTandensureMountsyncJWTVerifiernow taketesting.TBso the benchmarks can use them.
No version bump or release.
🤖 Generated with Claude Code
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