process_file() ran process_file_chunks() inside `with backup_io("read")`.
That block was meant to guard reading the *source* file, but the source
reads are already guarded individually by backup_io_iter(). The outer
wrapper additionally caught add_chunk()'s *repository* writes, so a critical
repository IO failure -- e.g. the repo running out of space during a pack
flush -- was wrapped into a per-file BackupOSError. Borg then only warned,
skipped the file, and continued, and create_inner() still committed the
archive via archive.save().
The result: `borg create` on an out-of-space repo printed a normal success
summary ("Error files: 0"), exited 0, and committed an archive that
references chunks which were never durably stored. `borg check` afterwards
reports "Missing file chunk detected" and `borg compact` reports "Repository
has N missing objects!" -- silent, unrestorable-backup data loss.
Drop the outer backup_io("read") wrapper. Source reads stay per-file
warnings (backup_io_iter is unchanged); repository OSErrors are now left
unwrapped and therefore critical, aborting create before archive.save()
runs, exactly as the BackupOSError docstring prescribes.
Reproduced on a space-limited macOS ramdisk (source > free space): before,
create exited 0 with a corrupt archive; after, create fails and commits
nothing, and the repository stays consistent across a create/delete/compact
churn matrix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
compact_pack() updated repository.chunks, but compact computes keep/drop from
and persists its own index, which kept pointing at the deleted packs. Also:
tolerate deleting already-gone packs, start rebuilt index entries as unused.
max_count=3 was a testing value. The default is now max_count=None,
max_size=50_000_000; the testsuite sets BORG_PACK_MAX_COUNT=3 instead,
so small test data still produces multiple multi-object packs.
close() persists the files cache but never drops its reference to
self._files, unlike self._chunks which is cleared right after being
saved. For a backup of many files this dict can hold hundreds of MB and
stays alive for as long as the closed cache object does.
Set self._files = None after writing it, mirroring the _chunks handling.
Accessing .files after close() was never supported anyway (close() also
tears down cache_config, which _read_files_cache needs), so this only
frees memory earlier.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AdHocWithFilesCache._chunks is the very same ChunkIndex object the
Repository holds (bound via the .chunks property). On close() we persist
it and clear the table in place (_maybe_write_chunks_cache(clear=True)),
then set self._chunks = None. But that only drops the *cache's* reference:
the Repository still points at the now-empty table and
is_chunk_index_loaded stays True.
A later access to repository.chunks in the same process (e.g.
delete_object doing self.chunks.get(id)) would then see a valid-looking
but empty index and conclude every chunk is gone, instead of rebuilding
from the repository. delete_chunkindex_from_repo() already guards its
analogous case with invalidate_chunk_index(); do the same here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_config/cache/data/runtime_dir now accept *subdirs and ensure_dir the
full path. get_keys_dir and get_security_dir use this instead of
hand-joining "keys"/"security", and the store cache in repository.py uses
get_cache_dir("storecache") instead of its own makedirs.
On filesystems with large allocation units like tmpfs with hugepages,
SEEK_DATA can report an offset before the actual first non-zero byte.
The test already verifies file contents and checks st_blocks to confirm
the file is stored sparsely, so only require that the first data region
is not past the expected content.
Signed-off-by: Zhou Qiankang <wszqkzqk@qq.com>
fill() is declared 'except 0' and always returns 1: errors propagate as
exceptions, so the 'if not self.fill(): return None' branches were
unreachable (and process() returning None would have crashed __next__
anyway). Call fill() plainly instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A wrong field count for a named algo (e.g. fastcdc,19,23,21 or
buzhash,19,23,21 with a forgotten field) used to fall through to the
old-style buzhash compat branch, which then crashed with a ValueError
from int('fastcdc') instead of raising a proper ArgumentTypeError.
- restrict the old-style compat branch to what it is for: four all-numeric
fields without an algorithm name (19,23,21,4095 still works).
- for buzhash64 and fastcdc, a wrong field count now raises an
ArgumentTypeError that spells out the expected format, which helps since
buzhash64 recently gained nc_level and fastcdc has no window field.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
For buzhash64 and fastcdc, chunker params like chunk_min == chunk_max passed
CLI validation, but then failed with an AssertionError when the chunker was
constructed (buzhash64 needs window_size + 2^chunk_min + 1 <= 2^chunk_max,
fastcdc needs 2^chunk_min + 1 <= 2^chunk_max). Check this at parse time and
raise a proper ArgumentTypeError instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a new "fastcdc" content-defined chunker selectable via --chunker-params.
It uses the FastCDC Gear rolling hash (fp = (fp << 1) + Gear[byte]), which is
window-less and cheaper per byte than buzhash's cyclic-polynomial update, so it
chunks noticeably faster (see "borg benchmark cpu" output), while producing
the same chunk-size distribution and deduplication.
The Gear table is keyed: it is derived from the repo id key via CSPRNG (own
"fastcdc" domain), exactly like the buzhash64 table, so chunk cut points stay
unpredictable without the key (anti-fingerprinting). It implements the same
FastCDC techniques as buzhash64 (sub-minimum skipping, normalized chunking with
a required nc_level, min/max clamping); the mask uses the high bits of the hash
(Gear accumulates entropy there).
chunker-params: "fastcdc,chunk_min,chunk_max,chunk_mask,nc_level" - there is no
window field, because Gear is window-less. e.g. fastcdc,19,23,21,2
Also: borg benchmark cpu now measures the fastcdc chunker; tests in
borg.testsuite.chunkers (golden vector, size distribution, keyed gear table,
param parsing, slow fuzz); docs and changelog.
Benchmarks (scripts/chunker_bench.py, buzhash64 vs fastcdc, both nc_level=2,
incompressible data unless noted):
5 GiB, 2 MiB target (default params):
buzhash64: CV 0.294, 1011 MB/s
fastcdc: CV 0.295, 1313 MB/s (+30%)
64 MiB, 64 KiB target:
buzhash64: CV 0.374, shift-resilience 0.9928, 963 MB/s
fastcdc: CV 0.359, shift-resilience 0.9929, 1331 MB/s (+38%)
Re-backup of a 2.5 GiB file after scattered single-byte edits (dedup ratio,
0.5 = v2 fully deduplicated, lower is better):
64 edits: buzhash64 0.5237, fastcdc 0.5236
320 edits: buzhash64 0.6133, fastcdc 0.6161
borg benchmark cpu, 1 GB: fastcdc 3.80s, buzhash 4.36s, buzhash64 8.13s,
fixed 0.56s.
Chunk-size distribution, deduplication and shift-resilience match buzhash64
within noise; fastcdc is consistently faster.
Also: fix bug when computing the mask, one needs to use 1ULL instead of
1, so the shifting computation is done in a uint64, not in a 32bit int.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lock_cmds, repo-create, repo-delete, repo-space and tar all only exercise
the generic store list/load/store/delete forwarding that every backend
implements identically:
- lock_cmds: Lock only does list/store/delete on locks/*, and its one
host-specific check (process_alive across hosts) isn't exercised
differently by this test infra regardless of "kind" anyway.
- repo-create/repo-delete: store.create()/destroy() are trivial 1:1 RPC
forwards in the REST backend; still exercised over the wire incidentally
via every remaining remote test file's setup (repo-create) and via
checks_test.py (repo-delete), which stays on remote.
- repo-space: grepped borg's own code - it never calls store.quota() at
all, only generic store_list/store/delete on config/space-reserve.*.
- tar cmds: do_export_tar/do_import_tar route through the same
Archive/Repository.get()/put() machinery as extract/create, so this
is redundant with their remote coverage (same category as recreate).
create, extract, check and transfer keep "remote" because they exercise
backends that genuinely branch per transport (partial reads/content-hash
upload headers, server-side hashing, two simultaneous store connections).
Rewrote the kinds-picking policy comment in generate_archiver_tests() to
state that rule directly instead of "moves real archive data".
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
check_cmd_test.py's remaining tests only exercise the generic store
list/load/delete calls also covered by other remote-tested files
(create, extract, ...), so drop remote from its kinds like #9324 did
elsewhere.
Repository.check() does have one transport-specific path though: the
REST backend computes pack hashes server-side (nothing downloaded),
unlike other backends. No existing test actually exercised a plain
--repository-only check catching a corrupted pack via that hash
comparison (existing corruption tests all go through --verify-data,
which decrypts client-side instead). Add that as its own local+remote
test.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The remote (rest://) archiver variant spawns a borgstore-server-rest
subprocess and does synchronous HTTP-over-stdio round trips per object,
so a remote test run costs several times its local twin (e.g.
test_prune_repository_example: ~2.8s local vs ~12.2s remote). Profiling
the suite showed every one of the 40 slowest tests was a remote_archiver
variant.
Most of those tests exercise pure command logic that operates on
metadata or only formats output (prune scheduling, diff/list/info
rendering, rename, delete/undelete, recreate, key handling, debug,
return codes, fuse mount) - this behaves identically regardless of
transport, and the data path over rest is already covered by the
create/extract/check/compact/repo/lock/transfer tests.
Switch those files to kinds="local,binary" and document the policy for
picking kinds in generate_archiver_tests().
Effect (local, -n auto, no coverage): remote_archiver invocations
374 -> 178; full suite 242s -> 154s (-36%), no test failures. The win
on the overloaded macOS CI runners (cold subprocess spawns + coverage)
should be relatively larger.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sphinxcontrib.jquery is already registered in the extensions list, so
the explicit app.setup_extension("sphinxcontrib.jquery") in setup() is
redundant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
