mirror of
https://github.com/borgbackup/borg.git
synced 2026-06-11 01:41:57 -04:00
compact: invalidate cached chunk indexes before deleting objects, #9748
If `borg compact` was interrupted after deleting repository objects but before writing the updated chunk index, the still-existing cache/chunks.* kept claiming the deleted objects were present. A later `borg create` would trust that stale index, skip re-uploading the affected chunks and silently create an archive with dangling object references that extracts to zero bytes. Invalidate all cached chunk indexes via delete_chunkindex_cache() before the first object is deleted, so an interruption is conservative: the next client rebuilds the index from actual repository contents and re-uploads any deleted data. The post-deletion save_chunk_index() still writes a fresh, valid index. Add a regression test covering both compact paths (default and --stats) that interrupts compaction right before save_chunk_index() and verifies no cached chunk index survives and a later create+extract reproduces the original bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
8509b3b7fa
commit
0b103291b8
3 changed files with 74 additions and 3 deletions
|
|
@ -185,6 +185,10 @@ New features:
|
|||
|
||||
Fixes:
|
||||
|
||||
- compact: invalidate cached chunk indexes before deleting objects, #9748.
|
||||
An interrupted compact no longer leaves a stale cache/chunks.* that claims
|
||||
deleted objects still exist, which could cause a later create to skip
|
||||
re-uploading data and silently produce an archive with dangling references.
|
||||
- files cache: fix no-change backup emptying the files cache, #9749
|
||||
- fix canonical_path() missing ':' before port number
|
||||
- fix: xattr xdg backup exclusion should be on 'false'
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ from pathlib import Path
|
|||
|
||||
from ._common import with_repository
|
||||
from ..archive import Archive
|
||||
from ..cache import write_chunkindex_to_repo_cache, build_chunkindex_from_repo
|
||||
from ..cache import write_chunkindex_to_repo_cache, build_chunkindex_from_repo, delete_chunkindex_cache
|
||||
from ..cache import files_cache_name, discover_files_cache_names
|
||||
from ..helpers import get_cache_dir
|
||||
from ..helpers.argparsing import ArgumentParser
|
||||
|
|
@ -177,6 +177,15 @@ class ArchiveGarbageCollector:
|
|||
if not (entry.flags & ChunkIndex.F_USED):
|
||||
unused.add(id)
|
||||
logger.info(f"Deleting {len(unused)} unused objects...")
|
||||
if unused:
|
||||
# Before deleting any repository object, invalidate all centrally cached chunk indexes.
|
||||
# Otherwise, if we get interrupted within the deletion loop, the still-existing cache/chunks.*
|
||||
# would claim that already-deleted objects are still present. A later "borg create" would then
|
||||
# trust that stale index, not re-upload the affected chunks and silently create an archive with
|
||||
# dangling object references (see issue #9748). By removing the cached indexes first, an
|
||||
# interruption is conservative: clients must rebuild the index from actual repository contents
|
||||
# and will re-upload any deleted data. save_chunk_index() writes a fresh, valid index afterwards.
|
||||
delete_chunkindex_cache(self.repository)
|
||||
pi = ProgressIndicatorPercent(
|
||||
total=len(unused), msg="Deleting unused objects %3.1f%%", step=0.1, msgid="compact.report_and_delete"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from ...constants import * # NOQA
|
||||
from ...helpers import get_cache_dir
|
||||
from ...cache import files_cache_name, discover_files_cache_names
|
||||
from . import cmd, create_src_archive, generate_archiver_tests, RK_ENCRYPTION
|
||||
from ...cache import files_cache_name, discover_files_cache_names, list_chunkindex_hashes
|
||||
from ...manifest import Manifest
|
||||
from . import cmd, create_regular_file, create_src_archive, generate_archiver_tests, open_repository, RK_ENCRYPTION
|
||||
from . import changedir
|
||||
|
||||
pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA
|
||||
|
||||
|
|
@ -85,6 +88,61 @@ def test_compact_index_corruption(archivers, request):
|
|||
assert "missing objects" not in output
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stats", (True, False))
|
||||
def test_compact_interrupted_does_not_poison_chunk_index(archivers, request, monkeypatch, stats):
|
||||
"""Regression test for issue #9748.
|
||||
|
||||
If a compact is interrupted after it deleted repository objects but before it wrote the
|
||||
updated chunk index, the still-existing cache/chunks.* must not claim that the deleted
|
||||
objects are still present. Otherwise a later "borg create" trusts the stale index, does
|
||||
not re-upload the affected chunks and silently produces an archive with dangling object
|
||||
references (which extracts to zero bytes).
|
||||
|
||||
The fix invalidates all cached chunk indexes before the first delete, so an interruption
|
||||
is conservative: the next client rebuilds the index from actual repository contents and
|
||||
re-uploads any deleted data. This is tested for both compact paths (default and --stats).
|
||||
"""
|
||||
from ...archiver.compact_cmd import ArchiveGarbageCollector
|
||||
|
||||
archiver = request.getfixturevalue(archivers)
|
||||
|
||||
# Unique content, so the chunk(s) become unused once we delete the only archive referencing them.
|
||||
content = os.urandom(1024 * 1024)
|
||||
create_regular_file(archiver.input_path, "file1", contents=content)
|
||||
|
||||
cmd(archiver, "repo-create", RK_ENCRYPTION)
|
||||
cmd(archiver, "create", "archive1", "input")
|
||||
cmd(archiver, "delete", "-a", "archive1")
|
||||
|
||||
# Simulate an interruption inside compact: run the real garbage collection (which deletes the
|
||||
# unused objects), but force it to abort right before it writes the fresh, updated chunk index.
|
||||
repository = open_repository(archiver)
|
||||
with repository:
|
||||
manifest = Manifest.load(repository, (Manifest.Operation.DELETE,))
|
||||
gc = ArchiveGarbageCollector(repository, manifest, stats=stats, iec=False)
|
||||
|
||||
def interrupt():
|
||||
raise KeyboardInterrupt("simulated interruption before save_chunk_index")
|
||||
|
||||
monkeypatch.setattr(gc, "save_chunk_index", interrupt)
|
||||
with pytest.raises(KeyboardInterrupt):
|
||||
gc.garbage_collect()
|
||||
|
||||
# The objects were deleted, so no readable cached chunk index may still list them.
|
||||
repository = open_repository(archiver)
|
||||
with repository:
|
||||
assert list_chunkindex_hashes(repository) == []
|
||||
|
||||
# A later backup of identical content must re-upload the deleted chunks, ...
|
||||
cmd(archiver, "create", "archive2", "input")
|
||||
# ... and extracting it must reproduce the original bytes without missing-object warnings.
|
||||
with changedir("output"):
|
||||
output = cmd(archiver, "extract", "archive2")
|
||||
assert "missing" not in output
|
||||
with open(os.path.join("input", "file1"), "rb") as fd:
|
||||
assert fd.read() == content
|
||||
|
||||
|
||||
def test_compact_files_cache_cleanup(archivers, request):
|
||||
"""Test that files cache files for deleted archives are removed during compact."""
|
||||
archiver = request.getfixturevalue(archivers)
|
||||
|
|
|
|||
Loading…
Reference in a new issue