Skip to content

Commit 4d970f4

Browse files
committed
check --repair: verify the written packs in two passes, refs #8466
Remove stale index entries of all written packs before indexing any object, so the result does not depend on the pack order. Look up each pack once, update the docs.
1 parent a7d2062 commit 4d970f4

7 files changed

Lines changed: 131 additions & 51 deletions

File tree

‎docs/internals/data-structures.rst‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ cache/
8686
a marker object: while it is present, the chunks index in ``index/`` is considered
8787
invalid, because its fragments may be missing entries or point at deleted packs.
8888
It is written before deleting index fragments, before a single-object delete removes
89-
the old pack, and before ``borg check --repair`` rebuilds the index after changing
90-
the packs. It is removed after the last fragment is deleted or once the complete
91-
current index is stored.
89+
the old pack, and by ``borg check --repair`` after storing packs, before it re-reads
90+
them and stores the index. It is removed after the last fragment is deleted or once
91+
the complete current index is stored.
9292

9393
Note that this ``cache/`` namespace is inside the repository (and thus shared by
9494
all clients); it is not the client-local cache described in

‎docs/internals/packs.rst‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,12 @@ A deletion that could drop entries -- dropping the index entirely, or the full r
323323
above -- is guarded by a marker object, ``cache/chunkindex-invalid``, written before
324324
the first deletion and removed after the last one. A single-object delete writes the
325325
marker just before it removes the old pack, and ``borg check --repair`` writes it
326-
before rebuilding the index after changing the packs; both remove it once the index
327-
is stored. While the marker is present, the fragments may be missing entries or point
328-
at deleted packs, so they are not merged; the index is rebuilt from the pack files on
329-
the next load instead. A consolidation needs no marker: the entries of the small
330-
fragments it deletes are already contained in the merged fragments it wrote before
331-
deleting them.
326+
after storing packs, before it re-reads them and stores the index; both remove it
327+
once the index is stored. While the marker is present, the fragments may be missing
328+
entries or point at deleted packs, so they are not merged; the index is rebuilt from
329+
the pack files on the next load instead. A consolidation needs no marker: the entries
330+
of the small fragments it deletes are already contained in the merged fragments it
331+
wrote before deleting them.
332332

333333
If the entire ``index/`` namespace is lost or corrupt, the ChunkIndex can be rebuilt
334334
by scanning pack files directly; see :ref:`pack-recovery`.

‎src/borg/archive.py‎

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,22 +2237,25 @@ class ArchiveChecker:
22372237
def __init__(self):
22382238
self.error_found = False
22392239
self.key = None
2240-
# True once repair wrote a pack: it stored a chunk or deleted a defect chunk.
2240+
# True once repair changed the packs: it stored a chunk or deleted a defect chunk.
22412241
self.chunks_modified = False
2242-
# ids of the existing packs repair stored (put(), flush()) or wrote by rewriting a pack (delete()).
2242+
# ids of the packs repair wrote: stored by put() and flush(), or written by delete() rewriting a pack.
22432243
self.written_packs = set()
22442244

22452245
def record_stored(self, results):
22462246
"""Add the pack ids in results to written_packs.
22472247
2248-
results: the (chunk_id, pack_id, obj_offset, obj_size) tuples Repository.put() or .flush() returns
2249-
for the packs it stored, or None if it stored no pack.
2248+
results: (chunk_id, pack_id, obj_offset, obj_size) tuples of the objects in the stored packs, as
2249+
Repository.put() and flush() return them, or None.
22502250
"""
22512251
if results:
22522252
self.written_packs.update(pack_id for _, pack_id, _, _ in results)
22532253

22542254
def create_archive_entry(self, name, id, ts):
2255-
"""Store the pack writer buffer, record the packs it wrote, create the archives directory entry."""
2255+
"""Store the pack writer buffer, record the packs it wrote, create the archives directory entry.
2256+
2257+
Archives.create() stores the pack writer buffer too, but does not return the packs it wrote.
2258+
"""
22562259
self.record_stored(self.repository.flush())
22572260
self.manifest.archives.create(name, id, ts)
22582261

@@ -2311,7 +2314,8 @@ def check(
23112314
self.repo_objs = RepoObj(self.key)
23122315
validate = object_validator(self.repo_objs)
23132316
# store the chunks buffered in the pack writer, so the index below has their pack locations
2314-
# (pack id, offset and size in the pack).
2317+
# (pack id, offset and size in the pack). The result is ignored: written_packs holds only the packs
2318+
# the repair writes.
23152319
self.repository.flush()
23162320
if not repair and self.repository.is_chunk_index_loaded:
23172321
# without --repair, use the loaded index.
@@ -2440,11 +2444,11 @@ def verify_data(self):
24402444
# keeping the other chunks, and removes it from self.chunks, so rebuild_archives
24412445
# reports the file it belongs to. update_index=False: finish() stores the index
24422446
# and clears the invalid marker delete() writes.
2443-
# new_pack_id holds the other objects of the old pack, None if there were none.
24442447
old_pack_id = self.chunks[defect_chunk].pack_id
2448+
# new_pack_id: the pack holding the other objects of the old pack, None if there were none.
24452449
new_pack_id, _ = self.repository.delete(defect_chunk, update_index=False, validate=validate)
24462450
self.chunks_modified = True
2447-
self.written_packs.discard(old_pack_id)
2451+
self.written_packs.discard(old_pack_id) # delete() removed the old pack
24482452
if new_pack_id is not None:
24492453
self.written_packs.add(new_pack_id)
24502454
else:
@@ -2813,48 +2817,61 @@ def valid_item(obj):
28132817
def verify_written_packs(self):
28142818
"""Read the object headers of the packs in written_packs and make the chunks index match them.
28152819
2816-
put() and delete() compute the index entries of the packs they write without reading the packs.
2820+
put() and delete() compute the index entries of the packs they write from the data they write.
28172821
This compares the (chunk_id, obj_offset, obj_size) of each object header in a written pack, read
2818-
with a validator, with the index entries that name the pack. Each difference is a check finding,
2819-
logged and fixed in the index:
2822+
with a validator, with the index entries that name the pack. Each difference sets error_found, is
2823+
logged and is fixed in the index:
28202824
28212825
- an index entry names an object the pack does not hold: the entry is removed.
28222826
- the pack holds an object whose chunk id is not indexed: the object is indexed.
28232827
- the pack does not exist: its index entries are removed.
28242828
2825-
An object whose chunk id is indexed at another location is a superseded duplicate, not a finding: a
2826-
pack delete() wrote can hold one, in a byte range compact_pack copied with no index entry covering it.
2829+
A superseded duplicate is an object whose chunk id is indexed at another location. It is logged at
2830+
debug level. A pack delete() wrote holds one if compact_pack copied a byte range that no index entry
2831+
covers.
28272832
"""
28282833
pack_ids = sorted(self.written_packs)
28292834
if not pack_ids:
28302835
return
2831-
logger.info(f"Re-reading the packs written by the repair: {len(pack_ids)}.")
2836+
logger.info(f"Re-reading {len(pack_ids)} pack(s) written by the repair.")
28322837
# (chunk_id, obj_offset, obj_size) of the index entries, per written pack.
28332838
indexed = {pack_id: set() for pack_id in pack_ids}
28342839
for chunk_id, entry in self.chunks.iteritems():
28352840
entries = indexed.get(entry.pack_id)
28362841
if entries is not None:
28372842
entries.add((chunk_id, entry.obj_offset, entry.obj_size))
28382843
validate = object_validator(self.repo_objs)
2844+
# pass 1 removes the index entries of every pack before pass 2 indexes any object, so whether an object
2845+
# is unindexed does not depend on the order the packs are read in.
2846+
found_in = {} # pack_id -> (chunk_id, obj_offset, obj_size) of the objects in the pack
2847+
not_found_in = {} # pack_id -> sorted index entries naming an object the pack does not hold
28392848
for pack_id in pack_ids:
28402849
# PackReader reads from the store, which does not refresh the repository lock.
28412850
self.repository._lock_refresh()
2842-
pack_hex = bin_to_hex(pack_id)
2843-
expected = indexed.pop(pack_id)
2844-
reader = PackReader(self.repository.store, pack_id)
2845-
# iter_headers() yields nothing for a missing pack: the store reports size 0 for it.
2846-
if not self.repository.store.info(reader.key).exists:
2851+
expected = indexed[pack_id]
2852+
key = "packs/" + bin_to_hex(pack_id)
2853+
info = self.repository.store.info(key)
2854+
if not info.exists:
28472855
self.error_found = True
2848-
logger.error(f"pack {pack_hex}: written by the repair, but it is missing. Removing its index entries.")
2856+
logger.error(
2857+
f"pack {bin_to_hex(pack_id)}: written by the repair, but it is missing. Removing its index entries."
2858+
)
28492859
for chunk_id, _, _ in expected:
28502860
del self.chunks[chunk_id]
28512861
continue
2862+
reader = PackReader(self.repository.store, pack_id, pack_size=info.size)
28522863
found = list(reader.iter_headers(validate=validate, on_drop=self.note_dropped_objects))
28532864
not_found = sorted(expected.difference(found))
28542865
for chunk_id, _, _ in not_found:
28552866
del self.chunks[chunk_id]
2856-
# the loop indexes each unindexed object, so of several unindexed copies of a chunk, the first
2857-
# is indexed and the others are superseded duplicates.
2867+
found_in[pack_id] = found
2868+
not_found_in[pack_id] = not_found
2869+
# pass 2 indexes each unindexed object, so of several unindexed copies of a chunk, the first in pack id and
2870+
# offset order is indexed and the others are superseded duplicates.
2871+
for pack_id, found in found_in.items():
2872+
pack_hex = bin_to_hex(pack_id)
2873+
expected = indexed[pack_id]
2874+
not_found = not_found_in[pack_id]
28582875
unindexed = []
28592876
for obj in found:
28602877
chunk_id, obj_offset, obj_size = obj
@@ -2874,6 +2891,8 @@ def verify_written_packs(self):
28742891
if not (not_found or unindexed):
28752892
continue
28762893
self.error_found = True
2894+
# an object the pack holds whose index entry has a wrong offset or size counts in both numbers,
2895+
# unless pass 2 indexed another copy of it first.
28772896
logger.error(
28782897
f"pack {pack_hex}: the chunks index does not match the pack. Indexed objects not in the pack: "
28792898
f"{len(not_found)}, objects in the pack with an unindexed chunk id: {len(unindexed)}. "
@@ -2892,13 +2911,13 @@ def finish(self):
28922911
if self.repair:
28932912
# store the pack writer buffer before the index is written (close() requires an empty buffer, #10055).
28942913
self.record_stored(self.repository.flush())
2895-
if self.chunks_modified:
2896-
# the index/ fragments do not have the chunks this repair stored.
2914+
if self.chunks_modified or self.written_packs:
2915+
# the index/ fragments lack the pack changes of this repair.
28972916
write_chunkindex_invalid(self.repository)
28982917
# Runs to completion, also after a Ctrl-C: delete_chunkindex_invalid() below declares the
28992918
# stored index to match the packs, which holds only once every written pack was re-read.
29002919
self.verify_written_packs()
2901-
logger.info("Writing the rebuilt repository chunks index.")
2920+
logger.info("Writing the repository chunks index.")
29022921
write_chunkindex_to_repo(
29032922
self.repository, self.chunks, incremental=False, clear=False, force_write=True, delete_other=True
29042923
)

‎src/borg/cache.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ def write_chunkindex_invalid(repository):
609609
"""Store the invalid marker, cache/chunkindex-invalid.
610610
611611
Store it before deleting index/ fragments whose entries no other fragment holds, before deleting a pack
612-
the fragments point at, and before rebuilding the index after pack changes the fragments do not record.
612+
the fragments point at, and after storing packs the fragments do not record.
613613
While it is present, build_chunkindex_from_repo rebuilds the index from the packs instead of merging the
614614
fragments.
615615
"""

‎src/borg/repository.py‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -406,14 +406,16 @@ def flush(self):
406406
class PackReader:
407407
"""Reads pack files, the read-side counterpart to PackWriter.
408408
409-
Pass pack_id to read from the store, or pack_contents for a pack already in memory.
409+
Pass pack_id to read from the store, or pack_contents for a pack already in memory. pack_size, if given,
410+
is the size of the pack in the store, so size() does not look it up.
410411
"""
411412

412-
def __init__(self, store=None, pack_id=None, pack_contents=None):
413+
def __init__(self, store=None, pack_id=None, pack_contents=None, pack_size=None):
413414
self.store = store
414415
self.pack_id = pack_id
415416
self.key = "packs/" + bin_to_hex(pack_id) if pack_id is not None else None
416417
self.pack_contents = pack_contents
418+
self.pack_size = pack_size
417419
self.headers_parsed = 0 # headers _parse_header accepted in the last iter_headers walk
418420

419421
def read(self, offset, size):
@@ -423,9 +425,11 @@ def read(self, offset, size):
423425
return self.store.load(self.key, offset=offset, size=size)
424426

425427
def size(self):
426-
"""Return the pack size in bytes (a store metadata lookup, unless the pack is in memory)."""
428+
"""Return the pack size in bytes (a store metadata lookup, unless the pack is in memory or pack_size is set)."""
427429
if self.pack_contents is not None:
428430
return len(self.pack_contents)
431+
if self.pack_size is not None:
432+
return self.pack_size
429433
return self.store.info(self.key).size
430434

431435
@staticmethod
@@ -498,8 +502,8 @@ def iter_headers(self, validate=None, on_drop=None):
498502
"""Yield (chunk_id, offset, size) for each object by walking the fixed object headers.
499503
500504
The walk reads one range per object (or a slice, for a pack in memory), plus one store
501-
metadata lookup for the pack size. Fewer than a header's bytes left ends the walk: that is
502-
the end of the pack.
505+
metadata lookup for the pack size unless pack_size is set. Fewer than a header's bytes left
506+
ends the walk: that is the end of the pack.
503507
504508
validate(chunk_id, obj) tells whether obj - an object's header and metadata slot - is the
505509
repo object with id chunk_id. Given one, the walk validates every header, reading the
@@ -1381,10 +1385,12 @@ def check(self, repair=False, max_duration=0, max_age=0, repo_only=False, valida
13811385
continuing. A read-only check never rebuilds the index: reading every pack to do so would be
13821386
far too slow and expensive for a routine (e.g. cron) check. With repair=True and a corrupt
13831387
index, and if every pack is intact, the index is rebuilt from the packs' object headers and
1384-
persisted. Packs are verified by the store hash, which is content-addressing rather than a MAC, so
1385-
that check detects accidental corruption but not tampering; the rebuild therefore checks every
1386-
object with validate, see below, refs #9901, #10026. If any pack is corrupt the index is left
1387-
unchanged, refs #8572, #10026. Pack ids found corrupt are kept in cache/checked-packs, refs #9696.
1388+
persisted; on a full check the archives phase rebuilds and re-persists it afterwards, see
1389+
ArchiveChecker.finish. Packs are verified by the store hash, which is content-addressing rather
1390+
than a MAC, so that check detects accidental corruption but not tampering; the rebuild therefore
1391+
checks every object with validate, see below, refs #9901, #10026. If any pack is corrupt the index
1392+
is left unchanged, refs #8572, #10026. Pack ids found corrupt are kept in cache/checked-packs,
1393+
refs #9696.
13881394
13891395
A pack recorded corrupt fails the check, also on a partial run that stops before re-reaching
13901396
it. The record clears at the check that finds the pack intact again or gone (removed by

0 commit comments

Comments
 (0)