create: overlap pack store and build of next pack, fixes #9988 - #10012
create: overlap pack store and build of next pack, fixes #9988#10012ThomasWaldmann wants to merge 2 commits into
Conversation
|
Real-world benchmark of the store-thread overlap: Data: unique random bytes interleaved 1:1 with zeros (≈2:1 lz4-compressible, no dedup effects), fresh repo per run,
Why these numbers: the pipeline turns
Upper bound for reference: a synthetic micro-benchmark with perfectly balanced simulated stages (0.2 s store per 1 MB pack vs matching per-chunk assembly cost) measures 1.75x, close to the theoretical 2x for one pack in flight. Summary: the gain ranges from ~nothing (very slow uplink + borg's fast assembly path, where there is simply little to hide) up to ~1.5x when assembly and store are comparable -- and async never measured slower than sync in any run. (benchmarks by Claude, reviewed by TW) |
PackWriter now hands a full pack to a background store-thread (at most one in flight): the pack bytes are joined, sha256-hashed (the pack_id) and stored in the store-thread, while the caller goes on assembling the next pack. hashlib and the store I/O release the GIL, so there is real overlap even on CPython. Throughput becomes max(assembly, store) instead of assembly + store. The ChunkIndex is only ever touched by the calling thread: the store-thread's results (or error) are applied when it is joined, at the next pack boundary or flush(). Consequences: - put()/add() return the *previous* pack's results while the current pack's store is in flight; update_pack_info() keys by chunk_id, so callers do not care which pack the results belong to. - flush() is a barrier: it joins an in-flight store and writes the current buffer synchronously, so afterwards nothing is F_PENDING anymore (needed by the periodic chunk index persist (#9900) and by close()). - a store error (e.g. ENOSPC) surfaces one pack later, from whichever add()/flush() call joins the store-thread - still before anything gets finalized, since the final flush is a barrier (no #9853-style regression). the failed pack's index entries are dropped and the buffered pieces die with the aborting command, so the close()-time index persist stays clean. - get()/get_many() of a chunk whose pack store is still in flight join the store-thread first (read barrier), then read normally. - close() joins a still-in-flight store (normally a no-op, flush ran before): a stored pack gets recorded, a failed one rolled back (not raising, to not mask the error being unwound). Sharing the Store between the store-thread and the main thread (lock refresh, reads of already stored packs) requires borgstore >= 0.6.0, which serializes all Store operations internally (borgstore #206 / #207). The borgstore dependency is bumped accordingly and now also pulls the blake3 extra, so borgstore's hash/defrag blake3 support is available server-side too. BORG_PACK_ASYNC=no disables the store-thread (debugging aid). The pre-existing synchronous-contract unit tests run with async_store=False; new tests cover deferred results, the combined flush barrier, deferred error surfacing with rollback, and the get() read barrier.
windows: 1.0.9 is broken py315: 1.0.8 is broken
ec6e460 to
c1ab5f1
Compare
Fixes #9988.
PackWriternow hands a full pack to a background store-thread (at most one in flight): the pack bytes are joined, sha256-hashed (the pack_id) and stored in the store-thread, while the caller goes on assembling the next pack. hashlib and the store I/O release the GIL, so there is real overlap even on CPython. Throughput becomesmax(assembly, store)instead ofassembly + store— with a simulated slow store (0.2s/pack) and balanced assembly work, the pipelining measured 1.75x end to end.The ChunkIndex is only ever touched by the calling thread: the store-thread's results (or error) are applied when it is joined, at the next pack boundary or
flush(). Consequences:put()/add()return the previous pack's results while the current pack's store is in flight;update_pack_info()keys by chunk_id, so callers do not care which pack the results belong to.flush()is a barrier: it joins an in-flight store and writes the current buffer synchronously, so afterwards nothing is F_PENDING anymore (needed by the periodic chunk index persist (borg2: does borg persist UNKNOWN pack ids in periodic index writes? #9900) and byclose()).add()/flush()call joins the store-thread — still before anything gets finalized, since the final flush is a barrier (no create: do not wrap repository writes in backup_io("read") (silent data loss on ENOSPC) #9853-style regression). The failed pack's index entries are dropped and the buffered pieces die with the aborting command, so the close()-time index persist stays clean.get()/get_many()of a chunk whose pack store is still in flight join the store-thread first (read barrier), then read normally.close()joins a still-in-flight store (normally a no-op,flush()ran before): a stored pack gets recorded, a failed one rolled back (not raising, to not mask the error being unwound).Sharing the
Storebetween the store-thread and the main thread (lock refresh, reads of already stored packs) requires borgstore >= 0.6.0, which serializes all Store operations internally (borgbackup/borgstore#206 / borgbackup/borgstore#207). The dependency is bumped accordingly and now also pulls theblake3extra.BORG_PACK_ASYNC=nodisables the store-thread (debugging aid, undocumented like the otherBORG_PACK_*tuning knobs).Tests: the pre-existing synchronous-contract unit tests run with
async_store=False; new tests cover deferred results, the combined flush barrier, deferred error surfacing with rollback, and theget()read barrier. Full local suite green (repository/cache/archiver), plus end-to-end create/check/extract/compact runs in both modes with tiny packs.Note (pre-existing, not addressed here): on master, an abort with chunks still buffered makes
close()'s "PackWriter has unflushed chunks" assert fire during unwind, masking the original exception — that happens with and without this PR and deserves its own fix.(implemented by Claude, reviewed by TW)
🤖 Generated with Claude Code