#361's fetch cache is not bounded by 4x the cap: one entry measures 62 MB against 32 MB on a wide varlena table, and gets 1.14x

[ChronicallyJD]

Raising this as an issue because #361 merged before my review landed, and a measured
defect should not live only in a closed PR thread. Numbers are unchanged from my
comment there; this adds nothing new except a place to track it.

Summary

#361 states, in the PR body and in design/ISSUE_359_FETCH_CACHE_PARTIAL.md, that
the per-column release keeps the fetch cache "bounded by 4 x the cap". It does not.
On a wide varlena table a single entry measures 62 MB against a 32 MB cap, and
nothing can bring it back down.

The same shape gets almost none of the speedup the change is for: 1.14x, against
the 13.6x #361 reports at its own crossing.

Measurement

Fixture: 150,000 rows x 60 text columns of 100 B plus a bigint key — one stripe at
the default stripe_row_limit, index on the key, forced index scan over 200 rows.
Every column varlena, so every one gets a valOffset. pg18n (-O2), one run at a
time on an idle box, main before #361 as the control.

Memory, from pg_backend_memory_contexts read inside the transaction so the
statement-scoped contexts are still alive:

columnar fetch column | n=2 | total=18 MB     <- only two columns stayed resident
columnar fetch group  | n=1 | total=44 MB     <- groupBuffer + the retained indexes
ALL columnar fetch contexts total: 62 MB

Timing, same fixture, same 200 fetches:

build	time
before #361	145,132 ms
after #361	127,699 ms

Why

Two things #361 makes non-releasable, plus a whole-entry drop that can no longer see
them:

1. rankPrefix[c] and valOffset[c] stay in entry->cx by design, so they
   outlive a released column. That part is right — without it Fetch by row number re-reads and re-decodes the whole row group, making UPDATE and DELETE quadratic within a group #143's quadratic
   returns through the overflow path, and the design doc argues it well.
2. But valOffset is 4 bytes per value per varlena column. At the default
   stripe_row_limit of 150,000 that is ~600 KB of permanently-resident state per
   varlena column, and 60 of them is ~34 MB — over the cap on its own, before a
   single decoded byte is retained. That is the 44 MB above.
3. The whole-entry drop now keys on raw bytes:
   if (rg->byteLength > COLUMNAR_FETCH_CACHE_MAX_BYTES). This text compresses well,
   so byteLength stays small and the drop never fires.

The design doc justifies retention with "valOffset is 4 bytes per value against a
value that is typically wider". That holds while the column is resident. Once it is
released the stream goes and the index stays, so the thing it is being compared
against is no longer there.

The 1.14x follows from the same fact: once the retained indexes alone exceed the cap,
MemoryContextMemAllocated(entry->cx, true) is over the limit on every decode, so
every newly decoded column is released immediately. Two columns got in before the
entry crossed; the other 58 overflow on every fetch. It degrades to roughly the
pre-#361 behaviour while now pinning 62 MB instead of dropping it.

Real ceiling is 4 x (cap + retained indexes + groupBuffer) — ~248 MB on this shape,
not 128 MB.

Fix direction

Cheapest first; I have measured none of them.

1. Release valOffset[c] for a column that has overflowed, keep rankPrefix[c].
   rankPrefix is 4 bytes per 64 rows (~9 KB per column here) against ~600 KB for
   valOffset, so this keeps the constant-time row reach that matters for Fetch by row number re-reads and re-decodes the whole row group, making UPDATE and DELETE quadratic within a group #143 at
   ~1.5% of the memory. The cost is that an overflowed varlena column re-walks its
   value stream to rebuild offsets — a constant factor on a decode that is already
   happening per fetch, not a return of the quadratic. This is the one I would try.
2. Count the retained indexes against the cap when deciding admission, so
   admission stops before retention eats the budget.
3. A whole-entry drop keyed on the entry's actual footprint rather than on raw
   bytes, as a backstop for whatever (1) and (2) do.

Whatever lands, the "bounded by 4 x the cap" sentence in the body and the matching
line in the design doc need to change, and the test wants to assert the bound
(read pg_backend_memory_contexts in-transaction) rather than only the timing ratio —
a timing test cannot distinguish "proportional" from "degraded back to total but now
holding memory".

Not a regression

Worth saying plainly: before #361 this shape pinned nothing and re-decoded
everything, so the memory ceiling is new but the time is not — 145 s to 128 s is an
improvement, just a marginal one. #361 is a clear win on the shapes it targets. This
is the shape it does not reach.

Provenance

Found while reviewing #361 (comment on the merged PR has the same numbers). Fixture
and method are in that comment; happy to take the patch.

[ChronicallyJD]

I built the fix I recommended and measured it. It is the wrong trade — withdrawing it.

I said option (1) — release valOffset[c] for an overflowed column, keep
rankPrefix[c] — was "the one I would try", on the reasoning that rebuilding offsets
is "a constant factor on a decode that is already happening". I flagged that as
unmeasured. It is measured now, and the reasoning was wrong.

Same fixture as the original report (150,000 rows x 60 text columns of 100 B, one
stripe, forced index scan over 200 rows, pg18n, one run at a time on an idle box):

build	memory held	time
before #361 (drop the whole entry)	nothing retained	145,132 ms
#361 as merged (retain everything)	62 MB vs a 32 MB cap	127,699 ms
my option (1) patch (release valOffset)	28 MB — under the cap	187,521 ms

The memory half works exactly as predicted: the entry context falls from 44 MB to
9,985 kB, total from 62 MB to 28 MB, and the bound holds. The time half is 47%
worse than #361 and 29% worse than the pre-#361 behaviour it was meant to improve on.
So the patch is worse than both alternatives on time while only fixing the bound.

Why the reasoning failed: the re-decode and the offset rebuild are both O(values), but
they are two separate walks plus a 600 KB allocation, per overflowed column, per
fetch — 58 columns x 200 fetches here. "Already happening anyway" was true of the
decode and not of the second walk, and I treated the two as one.

The retained offsets in #361 are not incidental overhead; they are what makes an
overflowed column cheap on the next fetch. Releasing them gives that up.

What this means for the issue

The bound and the speed are in genuine tension on this shape, and I no longer think
there is a free fix. Holding offsets for 60 varlena columns at a 150,000-row group
needs ~34 MB whatever we do; it does not fit in a 32 MB cap. The real options are:

1. Accept Hold what fits in the fetch cache instead of dropping it all (#359) #361's memory for its speed, and correct the "bounded by 4 x the cap"
   claim in the body and design doc to state the actual bound
   (cap + retained indexes + groupBuffer). Cheapest, honest, no code.
2. Bound it by dropping the entry when the retained footprint exceeds the cap,
   rather than when raw bytes do. That returns this shape to roughly the pre-Hold what fits in the fetch cache instead of dropping it all (#359) #361
   145 s and keeps memory bounded — a deliberate choice of the bound over the speed,
   which at least makes the cap mean what it says.
3. Make the group smaller for these shapes, i.e. treat a very wide varlena table
   as a reason to reduce the effective group the fetch path materialises. Bigger
   change, and I have not thought it through.

I lean (1) plus a documented bound, with (2) available if you would rather the cap be
a real ceiling. I do not think this is alpha-blocking either way — #361 is a clear
improvement over what preceded it on both axes that matter to users here.

Not opening a PR. The branch exists if you want to see it, but I am not proposing
it. Happy to do (1) as a docs-and-comment change or (2) as a patch, whichever you
prefer — your call, since it is your design decision that the numbers are now
informing rather than overturning.