repository: cache whole packs in get_many, replacing adjacent-read grouping

[mr-raj12]

What

Repository.get_many called store.load once per object. Objects in a pack file are stored back-to-back and a pack is immutable once written, so get_many now fetches each pack once and slices every requested object out of the in-memory copy. Repeated ids and reads that interleave several packs are served from memory instead of re-fetching.

How

For each requested id, get_many looks up the chunks index entry (pack_id, obj_offset, obj_size). The pack is loaded whole on a cache miss and kept in a per-repository borg.helpers.LRUCache of up to PACK_READER_CACHE_SIZE (3) packs, keyed by pack_id. Later ids whose pack is still cached are sliced from memory with no store request. The cache is cleared when the repository is closed.

get() peeks the same cache before doing a store range request, in all cases including read_data=False: if the object's pack is already cached it is sliced from memory, otherwise get() does ranged reads as before and never loads a whole pack itself. Ids with no index entry, or still buffered in the pack writer, delegate from get_many to get().

Why it matters

On backends with per-call latency (S3, SFTP, any network store), Archive.fetch_many -> Repository.get_many dominates extract time. Deduplication makes the same chunk id repeat: a sparse file writes many identical zero chunks in a row, and a block that recurs later reappears scattered through the list. Items also interleave objects from several packs. Caching whole packs collapses this to one store.load per distinct pack, regardless of request order or repeats. The earlier adjacent-run grouping only helped when the request order matched storage order with no repeats.

Tests

Tests in repository_test.py:

• test_get_many_one_load_per_pack: store.stats["load_calls"] delta equals the number of packs, not the number of objects.
• test_get_many_keeps_request_order: ids interleaved across two packs load each pack once and come back in request order.
• test_get_many_repeated_ids: a deduplicated list like [A, A, B, C, B] returns the right bytes at every position with one load per distinct pack.
• test_get_many_evicts_least_recently_used: visiting more packs than the cache holds evicts the oldest, and revisiting an evicted pack reloads it.
• test_get_many_missing_id_yields_none: with raise_missing=False, a missing id yields None in its position without disturbing neighbors.
• test_get_reuses_cached_pack: a cold get() does one ranged read and does not populate the cache; after get_many caches the pack, get() and get(read_data=False) do zero store loads.

[codecov]

Codecov Report

❌ Patch coverage is 77.14286% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.08%. Comparing base (5aaa2dd) to head (cc098ec).
⚠️ Report is 9 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/borg/repository.py	77.14%	7 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9836      +/-   ##
==========================================
- Coverage   85.11%   85.08%   -0.03%     
==========================================
  Files          93       93              
  Lines       15408    15436      +28     
  Branches     2326     2332       +6     
==========================================
+ Hits        13114    13134      +20     
- Misses       1596     1603       +7     
- Partials      698      699       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[ThomasWaldmann]

Hmm, if an items chunks list is [A, A, B, C, B], does this work?

[mr-raj12]

Hmm, if an items chunks list is [A, A, B, C, B], does this work?

https://gist.github.com/mr-raj12/830ebd466f3349739c534556a4486c5c

[mr-raj12]

the same id repeats in a get_many call, we end up reading it from the store again instead of reusing what we already have. Should I handle this dedup (with a proper memory limit) in this PR only, or would you prefer a separate follow-up?

[ThomasWaldmann]

Well, maybe i shouldn't have asked "does this work", but rather said "guess this is not how it should work".

Reading a pack from the store should go through a small LRU-cache of PackReaders.

A PackReader instance could have a mapping property that scans the pack on first access and remembers the mapping obj_id -> obj_offset, obj_size then (guess that is less expensive than scanning the whole chunks index for this information).

get_many should always try to get an object from a cached PackReader first, before requesting it from the store.

[ThomasWaldmann]

BTW, just saw this on twitter and found it funny:

https://x.com/Habibat456/status/2071938201501974692

[mr-raj12]

BTW, just saw this on twitter and found it funny:

https://x.com/Habibat456/status/2071938201501974692

literally me hanging on the wall 🥇

[ThomasWaldmann]

use the force! ehrm, the python stdlib. or borg.helpers.

[ThomasWaldmann]

Looks like this works!

But maybe we can even improve it further...

[ThomasWaldmann]

i only reviewed the repository code this time.

[ThomasWaldmann]

if get() does not find an OBJECT id in the index, it will raise ObjectNotFound (correct). this means we do not have it in the index and likely also not in a pack and there is no sign of index corruption.

if get_many() does not find a PACK, it raises the same exception, although this points to a bigger issue: we have an index entry that points to a pack, but that whole pack is missing, not just a single object. this either means the index is wrong or we have lost more data than a single object.

guess we need a different exception here, separate PR.

[mr-raj12]

follow-up PR