Skip to content

calculatedUids materializes the full uid list on the read path, ungated by the posting-list cache #9807

Description

@matthewmcneely

Summary

List.calculateUids() materializes the complete uid set of a posting list as an
uncompressed []uint64 on the read path, inside readFromDisk, before the caller's
ListOptions are known. Three problems follow from doing it there:

  1. It is not gated on the posting-list cache being enabled, so with
    --cache percentage=0,... the array is built, never stored, and discarded.
  2. It short-circuits several Uids() optimizations that v24.1.2 relied on, including the
    First early stop and the compressed-intersection fast path.
  3. Count-only queries pay for it and never read it.

Introduced in #9430. v24.1.2 has no equivalent path (readFromDisk there takes no
readUids argument).

Surfaced while triaging #9805, where an operator running percentage=0,80,20 reported
higher process_resident_memory_bytes after upgrading v24.1.2 to v25.3.8.

The path

worker/task.go:858 (also :2538 and :2584) calls LocalCache.GetUids, which threads
readUids=true down to readFromDisk:

worker/task.go:858        qs.cache.GetUids(key)
posting/lists.go:393      LocalCache.GetUids   -> getInternal(key, true, true)
posting/mvcc.go:850       getNew               -> MemLayerInstance.ReadData(..., readUids)
posting/mvcc.go:822       ReadData             -> readFromDisk(..., readUids)
posting/mvcc.go:795       readFromDisk         -> l.calculateUids()
posting/list.go:1726      calculateUids        -> l.iterate(committedUidsTime, 0, ...)

That iterate walks the whole list (mutable layer plus the packed immutable layer) with
afterUid=0, no count limit, and no intersect bounds, and appends every Posting_REF uid
into res.

1. Not gated on the posting-list cache

initMemoryLayer only builds a cache when cacheSize > 0
(posting/mvcc.go:514), and both Cache.get and Cache.set are nil-receiver-safe
(posting/mvcc.go:348, posting/mvcc.go:365). So when the posting-list cache share is
zero:

  • readFromCache always misses and returns nil.
  • readFromDisk still runs calculateUids().
  • saveInCache no-ops, so the array is dropped when the query's LocalCache goes away.

The materialization is a cache-warming optimization, but with the cache off there is
nothing to amortize it against. percentage=0,X,Y is a reasonable configuration for
deployments that would rather give the memory to badger's block cache, and it is exactly
the configuration that pays the most here.

2. Bypasses the Uids() optimizations

Uids() checks canUseCalculatedUids first (posting/list.go:1773), so when the array is
present the slow path below it never runs. That path is where the following live:

Optimization Location Effect when skipped
opt.First early stop posting/list.go:1849 first: 10 against a 10M-uid predicate materializes all 10M uids (~80 MB) instead of stopping at 10
opt.Intersect min/max narrowing posting/list.go:1841-1846 full walk instead of a bounded one
algo.IntersectCompressedWith posting/list.go:1804-1809 intersects against the packed encoding without decompressing; replaced by full materialization plus the generic intersect at posting/list.go:1864

The First case is the one I would expect to hurt most in practice, since paginated
queries over large predicates are common and the cost scales with the predicate rather
than with the page size.

3. Count queries never read it

For q.DoCount, worker/task.go:873 calls countForUidPostings, which goes through
facetsFilterUidPostingList to pl.Postings() (worker/task.go:730). That never touches
calculatedUids. compareScalarFn reaches pl.Length() at worker/task.go:884, same
story. Both already had the array built for them by GetUids.

Suggested directions

Any of these would help, and they compose:

  1. Gate readUids on the memory layer actually having a cache, so a zero-share
    posting-list cache stops paying for an array nobody keeps.
  2. Defer materialization out of readFromDisk and into Uids(), where opt.First and
    opt.Intersect are known, and only build the full array when the query really wants
    the whole list.
  3. Have handleUidPostings request uids only for the function types that consume them,
    rather than calling GetUids for count and compare-scalar paths too.

I have not benchmarked the delta, so I would want a posting-package benchmark over a
large predicate (paginated read, small-intersect read, and count) to size each of these
before picking one.

Notes

The correctness of calculatedUids itself is not in question here. #9801 already fixed
the read-timestamp leak, and canUseCalculatedUids (posting/list.go:1753) looks right.
This is about where the array gets built and who pays for it.

Related: #9805, #9430, #9801

Metadata

Metadata

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions