Skip to content

Fetch packed objects with ranged reads instead of whole packfiles #451

Description

@rmanibus

Context

PackStore.Get resolves a packed object by downloading the entire packfile
and caching it in a 4-entry LRU (internal/storelayer/pack.go):

	s.debugf("get %s: downloading pack %s", key, entry.PackRef)
	// 4. Download the entire packfile, cache it, and return the slice
	packData, err := s.ObjectStore.Get(ctx, entry.PackRef)
	...
	s.packCache.Add(entry.PackRef, packData)

There is no ranged read on this path, even though the catalog entry carries the
exact Offset and Length, and even though readPackFooter already uses the
optional store.RangeGetter interface for precisely this reason —
TestPackStore_UsesRangedReadsForFooters exists to stop the footer path
regressing to whole-pack transfers. The object path never had that treatment.

So retrieving one ~200-byte filemeta from a cold pack transfers up to
maxPackSize (8 MB). With packCache bounded at 4 packs, any access pattern
whose working set exceeds four packs re-transfers them repeatedly.

This is invisible on small repositories — a 50,000-file tree produces only 4
packs, so the cache holds everything. It stops being invisible as soon as pack
count exceeds the cache, and it compounds with access order.

Tracing done for RFC 0023 (#440) measured that order. Recording which pack each
catalog resolution lands in, on a 50,000-file tree configured to produce 51
packs, restore misses a 4-entry LRU on 46.9% of its 109,597 lookups. Its
write phase is the cause: topoSort orders parent-before-child, which bears no
relation to pack layout, giving 55.5% misses against 0.6% for the metadata fetch
phase. On a remote backend each of those misses is a whole packfile.

Goal

Retrieving a packed object transfers the object, not the packfile that contains
it, on any backend that supports ranged reads.

Scope

  • Use store.RangeGetter in PackStore.Get to fetch entry.Offset/entry.Length
    directly, falling back to the current whole-pack Get for backends without it,
    mirroring what readPackFooter already does.
  • Decide what the body cache is for once ranged reads exist. Caching whole packs
    to serve small slices is only worthwhile where locality is high; it may be
    better to cache decoded objects, keep the pack cache for the sequential case,
    or size it by bytes rather than pack count.
  • Keep the existing bounds checks: a ranged read must still fail when the pack is
    shorter than Offset+Length rather than returning a short object.
  • Consider whether Repack and any other whole-pack reader should share the path.

Acceptance Criteria

  • Retrieving one small object from a cold pack transfers approximately that
    object's bytes on a RangeGetter backend, verified the way
    TestPackStore_UsesRangedReadsForFooters verifies the footer path.
  • Backends without RangeGetter still work, via the whole-pack fallback.
  • A pack shorter than Offset+Length is still an error.
  • storetest.AssertRangeGetterConformance still passes for every backend.
  • go test ./internal/storelayer passes
  • golangci-lint run ./internal/storelayer/... passes

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/storeperfPerformance, memory, and scaling work

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions