From 2e2ef6f47aed6352cc68f5a81f178ab8bae06036 Mon Sep 17 00:00:00 2001 From: Mrityunjay Raj Date: Sat, 4 Jul 2026 22:13:04 +0530 Subject: [PATCH] repository: peek the pack cache in get(), switch to peekable helpers LRUCache --- src/borg/repository.py | 32 +++++++++++++++++---------- src/borg/testsuite/repository_test.py | 26 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/borg/repository.py b/src/borg/repository.py index 0e1a8dff1..d73ce7a97 100644 --- a/src/borg/repository.py +++ b/src/borg/repository.py @@ -1,4 +1,3 @@ -import functools import os import sys import time @@ -19,6 +18,7 @@ from .helpers import Location from .helpers import bin_to_hex, hex_to_bin from .helpers import get_cache_dir from .helpers import ProgressIndicatorPercent +from .helpers.lrucache import LRUCache from .storelocking import Lock from .logger import create_logger from .manifest import NoManifestError @@ -392,8 +392,8 @@ class Repository: self.exclusive = exclusive self._pack_writer = None self._chunks = None # ChunkIndex; loaded lazily on first access to .chunks - # pack_id -> PackReader, per repository so close() can free the cached packs - self._cached_pack_reader = functools.lru_cache(maxsize=self.PACK_READER_CACHE_SIZE)(self._load_pack_reader) + # pack_id -> PackReader holding the whole pack; get_many loads into it, get() reuses it + self._pack_cache = LRUCache(capacity=self.PACK_READER_CACHE_SIZE) def __repr__(self): return f"<{self.__class__.__name__} {self._location}>" @@ -601,7 +601,7 @@ class Repository: self.store.close() self.store_opened = False self.opened = False - self._cached_pack_reader.cache_clear() + self._pack_cache.clear() def info(self): """return some infos about the repo (must be opened first)""" @@ -768,10 +768,14 @@ class Repository: raise self.PackLocationUnknown(id, str(self._location)) pack_id, obj_offset, obj_size = entry.pack_id, entry.obj_offset, entry.obj_size id_hex = bin_to_hex(id) - key = "packs/" + bin_to_hex(pack_id) + # slice from the cached whole pack if get_many (or an earlier get) already loaded it; + # otherwise read ranges from the store without loading and caching the whole pack. + reader = self._pack_cache.get(pack_id) + if reader is None: + reader = PackReader(store=self.store, pack_id=pack_id) try: if read_data: - return self.store.load(key, offset=obj_offset, size=obj_size) + return reader.read(obj_offset, obj_size) else: # RepoObj layout supports separately encrypted metadata and data. # We return enough bytes so the client can decrypt the metadata. @@ -783,7 +787,7 @@ class Repository: # uses the header's length and ignores trailing bytes -- this is just tidy.) obj_size # comes from the same index we already route with. load_size = min(load_size, obj_size) - obj = self.store.load(key, offset=obj_offset, size=load_size) + obj = reader.read(obj_offset, load_size) hdr = obj[0:hdr_size] if len(hdr) != hdr_size: raise IntegrityError(f"Object too small [id {id_hex}]: expected {hdr_size}, got {len(hdr)} bytes") @@ -794,7 +798,7 @@ class Repository: retry_size = hdr_size + meta_size # same boundary as above: normally a no-op, just keeps the retry within this object. retry_size = min(retry_size, obj_size) - obj = self.store.load(key, offset=obj_offset, size=retry_size) + obj = reader.read(obj_offset, retry_size) meta = obj[hdr_size : hdr_size + meta_size] if len(meta) != meta_size: raise IntegrityError(f"Object too small [id {id_hex}]: expected {meta_size}, got {len(meta)} bytes") @@ -805,10 +809,14 @@ class Repository: else: return None - def _load_pack_reader(self, pack_id): - """Return a PackReader holding the whole pack, loaded from the store.""" - key = "packs/" + bin_to_hex(pack_id) - return PackReader(pack_id=pack_id, pack_contents=self.store.load(key)) + def _cached_pack_reader(self, pack_id): + """Return a PackReader holding the whole pack, loading it into the cache on a miss.""" + reader = self._pack_cache.get(pack_id) + if reader is None: + key = "packs/" + bin_to_hex(pack_id) + reader = PackReader(pack_id=pack_id, pack_contents=self.store.load(key)) + self._pack_cache[pack_id] = reader + return reader def get_many(self, ids, read_data=True, raise_missing=True): if not read_data: diff --git a/src/borg/testsuite/repository_test.py b/src/borg/testsuite/repository_test.py index 7a62d585c..be43d3de1 100644 --- a/src/borg/testsuite/repository_test.py +++ b/src/borg/testsuite/repository_test.py @@ -288,6 +288,32 @@ def test_get_many_evicts_least_recently_used(repo_fixtures, request): assert repository.store.stats["load_calls"] - loads_before == 5 # 4 distinct packs + 1 reload +def test_get_reuses_cached_pack(repo_fixtures, request): + # get() slices from a pack that get_many already cached, doing no store load; a get() whose pack + # is not cached reads only its object's range and does not load the whole pack into the cache. + objects = [(H(i), fchunk(b"payload-%02d" % i, chunk_id=H(i))) for i in range(2)] + with get_repository_from_fixture(repo_fixtures, request) as repository: + repository._pack_writer.max_count = 2 # one pack: {H0, H1} + for chunk_id, chunk in objects: + repository.put(chunk_id, chunk) + + # cold cache: get() reads one range and leaves the cache empty, so the next get() reads again + loads_before = repository.store.stats["load_calls"] + reference = repository.get(H(0)) + assert repository.store.stats["load_calls"] - loads_before == 1 # one ranged read + loads_before = repository.store.stats["load_calls"] + repository.get(H(1)) + assert repository.store.stats["load_calls"] - loads_before == 1 # pack was not cached, another range + + list(repository.get_many([H(0), H(1)])) # loads the whole pack into the cache + + loads_before = repository.store.stats["load_calls"] + assert repository.get(H(0)) == reference + assert repository.store.stats["load_calls"] - loads_before == 0 # sliced from the cached pack + assert repository.get(H(1), read_data=False) # read_data=False also peeks the cache + assert repository.store.stats["load_calls"] - loads_before == 0 + + def build_one_pack(repository, objects): with repository: repository._pack_writer.max_count = len(objects) + 1 # prevent per-put flush; one pack on flush()