From bf36090497a8509ceba8c2e97d5faab48dfeed78 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Mon, 22 Jun 2026 19:03:13 +0530 Subject: [PATCH] repository: compact_pack takes sets, unifies keep/drop loop, renames kept keep_ids/drop_ids are now keyword-only sets; one loop over their union replaces two; survivors renamed to kept; intersection asserted empty. --- src/borg/repository.py | 40 +++++++++++++-------------- src/borg/testsuite/repository_test.py | 23 ++++++++------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index fa708a88b..512f99672 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -810,12 +810,13 @@ class Repository: raise self.ObjectNotFound(id, str(self._location)) logger.warning("ignoring deletion of %s in %s", bin_to_hex(id), bin_to_hex(entry.pack_id)) - def compact_pack(self, pack_id, keep_ids, drop_ids): + def compact_pack(self, pack_id, *, keep_ids: set, drop_ids: set): """Rewrite pack , keeping and dropping , then delete the old pack. - keep_ids and drop_ids must together cover the whole pack (asserted: their ranges tile it with no - gap or overlap). Kept objects are copied into a new pack via store.defrag and repointed in the - chunk index; dropped objects' index entries are removed. + keep_ids and drop_ids are sets of chunk ids that must together cover the whole pack (asserted: + their ranges tile it with no gap or overlap, and their intersection is empty). Kept objects are + copied into a new pack via store.defrag and repointed in the chunk index; dropped objects' index + entries are removed. Returns the new pack_id, None if nothing is kept (pack dropped), or unchanged if the kept objects reproduce the old pack (same sha256 name, nothing to delete). @@ -826,43 +827,42 @@ class Repository: self._lock_refresh() pack_key = "packs/" + bin_to_hex(pack_id) + assert keep_ids & drop_ids == set(), "an id cannot appear in both keep_ids and drop_ids" + # collect every object's range, tagged with whether it is kept, ordered by offset. located = [] # (obj_offset, obj_id, obj_size, keep) - for keep_id in keep_ids: - entry = self.chunks[keep_id] - assert entry.pack_id == pack_id, f"{bin_to_hex(keep_id)} is not in pack {bin_to_hex(pack_id)}" - located.append((entry.obj_offset, keep_id, entry.obj_size, True)) - for drop_id in drop_ids: - entry = self.chunks[drop_id] - assert entry.pack_id == pack_id, f"{bin_to_hex(drop_id)} is not in pack {bin_to_hex(pack_id)}" - located.append((entry.obj_offset, drop_id, entry.obj_size, False)) + for obj_id in keep_ids | drop_ids: + keep = obj_id in keep_ids + entry = self.chunks[obj_id] + assert entry.pack_id == pack_id, f"{bin_to_hex(obj_id)} is not in pack {bin_to_hex(pack_id)}" + located.append((entry.obj_offset, obj_id, entry.obj_size, keep)) located.sort() - # keep + drop must tile the whole pack; pick out the survivors in the same pass. - survivors = [] # (obj_offset, obj_id, obj_size), offset-ordered + # keep + drop must tile the whole pack; collect the objects to keep in the same pass. + kept = [] # (obj_offset, obj_id, obj_size), offset-ordered covered = 0 for offset, obj_id, size, keep in located: assert offset == covered, f"gap or overlap in pack {bin_to_hex(pack_id)} at offset {covered}" covered += size if keep: - survivors.append((offset, obj_id, size)) + kept.append((offset, obj_id, size)) assert covered == self.store.info(pack_key).size, f"pack {bin_to_hex(pack_id)} not fully covered" for drop_id in drop_ids: # remove dropped objects from the index; their bytes are not copied forward del self.chunks[drop_id] - if not survivors: # nothing kept: drop the pack, no replacement + if not kept: # nothing kept: drop the pack, no replacement self.store_delete(pack_key) return None - # copy survivors into a new pack (named sha256 of its content) - sources = [(bin_to_hex(pack_id), offset, size) for offset, _, size in survivors] + # copy kept objects into a new pack (named sha256 of its content) + sources = [(bin_to_hex(pack_id), offset, size) for offset, _, size in kept] new_pack_id = hex_to_bin(self.store.defrag(sources, algorithm="sha256", namespace="packs")) - # repoint survivors at the new pack; new offset is the running sum of kept sizes + # repoint kept objects at the new pack; new offset is the running sum of kept sizes new_locations = [] offset = 0 - for _, keep_id, size in survivors: + for _, keep_id, size in kept: new_locations.append((keep_id, new_pack_id, offset, size)) offset += size self.chunks.update_pack_info(new_locations) diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 7776fd33e..74edb40fb 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -143,12 +143,11 @@ def test_consistency(repo_fixtures, request): def build_one_pack(repository, objects): - # Bundle several objects (N>1) into one pack: max_count == object count makes the last put flush the - # whole batch as a single pack. Caller reopens afterwards to read it back from disk. with repository: - repository._pack_writer.max_count = len(objects) + repository._pack_writer.max_count = len(objects) + 1 # prevent per-put flush; one pack on flush() for chunk_id, chunk in objects: repository.put(chunk_id, chunk) + repository.flush() def test_compact_pack_copy_forward(repo_fixtures, request): @@ -158,12 +157,12 @@ def test_compact_pack_copy_forward(repo_fixtures, request): chunk2 = fchunk(b"DATA2", chunk_id=H(2)) repository = get_repository_from_fixture(repo_fixtures, request) build_one_pack(repository, [(H(0), chunk0), (H(1), chunk1), (H(2), chunk2)]) - with reopen(repository) as repository: + with repository: old_pack_id = repository.chunks[H(0)].pack_id assert repository.chunks[H(1)].pack_id == old_pack_id assert repository.chunks[H(2)].pack_id == old_pack_id - new_pack_id = repository.compact_pack(old_pack_id, keep_ids=[H(0), H(2)], drop_ids=[H(1)]) + new_pack_id = repository.compact_pack(old_pack_id, keep_ids={H(0), H(2)}, drop_ids={H(1)}) assert new_pack_id is not None and new_pack_id != old_pack_id assert pdchunk(repository.get(H(0))) == b"DATA0" @@ -171,19 +170,19 @@ def test_compact_pack_copy_forward(repo_fixtures, request): assert repository.get(H(1), raise_missing=False) is None # compact_pack removed its index entry packs = {info.name: info.size for info in repository.store_list("packs")} assert bin_to_hex(old_pack_id) not in packs - assert packs[bin_to_hex(new_pack_id)] == len(chunk0) + len(chunk2) # only the survivors' bytes + assert packs[bin_to_hex(new_pack_id)] == len(chunk0) + len(chunk2) # only the kept objects' bytes def test_compact_pack_drops_whole_pack(repo_fixtures, request): - # Dropping every object removes the pack and clears its index entries. N>1 pack, not the max_count default. + # Dropping every object removes the pack and clears its index entries. chunk0 = fchunk(b"DATA0", chunk_id=H(0)) chunk1 = fchunk(b"DATA1", chunk_id=H(1)) repository = get_repository_from_fixture(repo_fixtures, request) build_one_pack(repository, [(H(0), chunk0), (H(1), chunk1)]) - with reopen(repository) as repository: + with repository: old_pack_id = repository.chunks[H(0)].pack_id - assert repository.compact_pack(old_pack_id, keep_ids=[], drop_ids=[H(0), H(1)]) is None + assert repository.compact_pack(old_pack_id, keep_ids=set(), drop_ids={H(0), H(1)}) is None assert repository.get(H(0), raise_missing=False) is None assert repository.get(H(1), raise_missing=False) is None @@ -192,15 +191,15 @@ def test_compact_pack_drops_whole_pack(repo_fixtures, request): def test_compact_pack_keep_all_is_noop(repo_fixtures, request): # Keeping every object reproduces the same pack: same sha256 name, old pack not deleted. Ids passed - # out of order must give the same result, since compact_pack sorts survivors by offset. + # out of order must give the same result, since compact_pack sorts by offset. chunk0 = fchunk(b"DATA0", chunk_id=H(0)) chunk1 = fchunk(b"DATA1", chunk_id=H(1)) repository = get_repository_from_fixture(repo_fixtures, request) build_one_pack(repository, [(H(0), chunk0), (H(1), chunk1)]) - with reopen(repository) as repository: + with repository: old_pack_id = repository.chunks[H(0)].pack_id - new_pack_id = repository.compact_pack(old_pack_id, keep_ids=[H(1), H(0)], drop_ids=[]) # out of order + new_pack_id = repository.compact_pack(old_pack_id, keep_ids={H(1), H(0)}, drop_ids=set()) # out of order assert new_pack_id == old_pack_id assert pdchunk(repository.get(H(0))) == b"DATA0"