posting: a dropped UID-warm publish re-arms immediately, so a read-hot and write-hot key loops on a discarded walk
What
The UID-slice warm added in #9809 discards its work and immediately re-arms when a commit lands mid-walk, so a posting-list key that is both read-hot and write-hot can loop on a walk whose result is always thrown away.
Where
posting/mvcc.go, warmCachedUids:
- A reader elects itself with
tryStartUidWarm, walks a private copy, and callspublishCalculatedUids. publishCalculatedUids(posting/list.go) drops the result whenl.mutationMap.committedUidsTimehas moved since the copy was taken.- The
defer cached.finishUidWarm()then returns the list touidWarmIdle, andisUidsCalculatedon the published list is still false, soneedsUidWarm()is still true and the next reader walks again.
Every commit on a cached key moves committedUidsTime: updateItemInCache calls setMutationAfterCommit(..., refresh=true), which bumps it via x.Max and clears isUidsCalculated/calculatedUids.
TestPublishCalculatedUidsDropsAStaleWalk covers the drop. Nothing bounds the retry.
Cost
Each discarded walk is a full l.iterate, a readListPart Badger read per split for a multi-part list, and a make([]uint64, 0, l.approxLen()).
It is bounded to one walk in flight per key, not one per read: concurrent readers lose the CAS and serve their read unwarmed without walking. So the shape is a single goroutine walking that key continuously and achieving nothing, rather than a per-read tax.
For contrast, before #9809's non-blocking warm the walk held the published list's write lock, which serialized it against the commit path — so the result was always installed and walks per key were bounded by commits. The trade #9809 made was "no commit stall" in exchange for this, and the commit message only described the first half. Worth correcting the record here.
Why it is not a one-liner
Two approaches that look obvious and are not:
- Park the list on a dropped publish (reuse
uidWarmAbandoned). A drop means a commit already landed, and that commit'sCompareAndSwap(uidWarmAbandoned, uidWarmIdle)ran before the park, so nothing re-arms it. A key that stops being committed then stays unwarmed for the life of the cache entry — and an entry with few deltas may never be rolled up and evicted, so "for the life of the entry" can mean indefinitely. - A per-key attempt throttle, the shape
doRollupuses (posting/mvcc.go: amap[uint64]int64ofz.MemHash(key)to last-attempt seconds, skipping anything attempted within 10s). That bounds it correctly and would bound the failure log for free, butdoRollupruns on one goroutine while this would run on every posting read, so the mutex would serialize reads across all keys. It needs sharding or something cheaper.
Not urgent
Reads stay correct throughout — an unwarmed read walks the list in Uids() exactly as it did before the memoization existed. This is wasted work, not a wrong answer.
Introduced in #9809 (b90c1800, perf(posting): warm cached UID slices off the published list's write lock).
Source: dgraph-io/dgraph