Re: pg_class.reltuples can become non-finite and never recovers

On 7/24/26 15:55, Tomas Vondra wrote:
> On 7/20/26 15:02, Tomas Vondra wrote:
>> On 7/20/26 14:40, Jan Nidzwetzki wrote:
>>> Hi hackers,
>>>
>>> We recently tracked down an issue where pg_class.reltuples for a
>>> heap relation had become Infinity, which in turn produced
>>> cost=Infinity / NaN query plans. While investigating, we found a
>>> chain of issues around reltuples that we would like to report, plus
>>> two patches that address the parts we consider clear bugs.
>>>
>>>
>>> 1. Uninitialized tuple counts in the parallel GIN build
>>> -------------------------------------------------------
>>>
>>> The root cause is in the parallel GIN build. _gin_parallel_build_main()
>>> sets up a fresh GinBuildState in the worker but, unlike ginbuild(),
>>> never initializes bs_numtuples or bs_reltuples. bs_numtuples happens to
>>> be reset before use, but bs_reltuples is not: the worker accumulates its
>>> scanned tuple count into it and publishes it to the leader, which stores
>>> it as the heap relation's reltuples via index_update_stats().
>>>
>>> The value read back is therefore whatever happened to be on the stack,
>>> and the store goes through an unchecked narrowing cast:
>>>
>>> rd_rel->reltuples = (float4) reltuples; /* index.c:2960 */
>>>
>>> The existing guard in index_update_stats() only rejects reltuples < 0,
>>> which a large positive or +Infinity double passes. Any stack residue
>>> whose double representation is >= FLT_MAX therefore lands in pg_class as
>>> +Infinity; smaller residue lands as a bogus-but-finite count that is
>>> unrelated to relpages.
>>>
>>> This was introduced together with the parallel GIN build:
>>>
>>> commit 8492feb98f6df3f0f03e84ed56f0d1cbb2ac514c
>>> "Allow parallel CREATE INDEX for GIN indexes"
>>>
>>> which first appeared in v18.
>>>
>>> Fix in the first attached patch: initialize bs_numtuples and
>>> bs_reltuples to 0 in the worker, matching how the leader's state is
>>> set up. We recommend back-patching this to 18.
>>>
>>
>> Yeah, this seems like my bug. I guess I got confused regarding what
>> fields get initialized, and how the values propagate. I suppose we could
>> also just change
>>
>> state->bs_reltuples += reltuples;
>>
>> to
>>
>> state->bs_reltuples = reltuples;
>>
>> And that'd fix the issue. But I agree it's the explicit initialization
>> is better / cleaner.
>>
>> I'll get this fixed and backpatched.
>>
>
> Here's a proposed fix - it's pretty much your patch, except that I
> removed the second initialization of the bs_numtuples field (which would
> be confusing, IMHO). And a slightly expanded commit message.
>
> There's not much to discuss, it's pretty clear (both the bug / fix).
>
>>>
>>> 2. VACUUM can amplify a bad reltuples/relpages ratio to +Infinity
>>> -----------------------------------------------------------------
>>>
>>> Independently of how the bad value first appears, it can get worse
>>> during normal operation. Once reltuples/relpages is inconsistent, a
>>> partial VACUUM extrapolates the stored density over the unscanned
>>> pages in vac_estimate_reltuples():
>>>
>>> old_density * unscanned_pages + scanned_tuples
>>>
>>> and the result is again stored through an unchecked (float4) cast in
>>> vac_update_relstats(). A merely large-but-finite reltuples can thus
>>> flip to +Infinity after a routine VACUUM. We mention this mostly to
>>> explain why the value tends to degrade toward Infinity in the field
>>> rather than staying at a "just wrong" finite number.
>>>
>>> This is easy to demonstrate starting from statistics whose individual
>>> values are perfectly valid but whose ratio is not: relpages = 1 and
>>> reltuples = 3e38 are each finite, yet together they imply an
>>> impossible density of 3e38 tuples per page:
>>>
>>> -+-
>>> CREATE TABLE vac_inf (a int);
>>> INSERT INTO vac_inf SELECT generate_series(1, 100000);
>>> VACUUM ANALYZE vac_inf;
>>>
>>> SELECT pg_restore_relation_stats(
>>> 'schemaname', 'public', 'relname', 'vac_inf',
>>> 'version', 190000,
>>> 'relpages', 1::integer, -- finite, valid
>>> 'reltuples', '3e38'::real); -- finite, valid, near FLT_MAX
>>>
>>> INSERT INTO vac_inf SELECT generate_series(1, 10000);
>>> VACUUM vac_inf;
>>>
>>> SELECT relpages, reltuples FROM pg_class WHERE relname = 'vac_inf';
>>> relpages | reltuples
>>> ----------+-----------
>>> 487 | Infinity
>>> (1 row)
>>> -+-
>>>
>>> The single VACUUM extrapolates the 3e38-per-page density over the
>>> newly added pages and overflows float4.
>>>
>>
>> Yeah. Garbage in, garbage out.
>
> Yeah. It's kinda orthogonal to the bug,, but I wonder if there's
> something we can do to mitigate this. But it's hard, if we don't know
> which of the values is wrong (if any).
>
> I guess we could refuse to update reltuples to non-finite value, but
> that doesn't make the existing reltuples value any less bogus. And I'm
> not sure it's less harmful than infinity in practice.
>
>
>>> 3. Infinity defeats autovacuum's self-healing
>>> ----------------------------------------------
>>>
>>> Normally, a wrong reltuples is self-correcting: ANALYZE recomputes it
>>> from a fresh sample. But a non-finite reltuples makes the analyze
>>> threshold non-finite as well:
>>>
>>> anlthresh = analyze_threshold + analyze_scale_factor * reltuples
>>>
>>> so in relation_needs_vacanalyze() the test "anltuples > anlthresh" can
>>> never be true, autoanalyze never fires, and the bad value is stuck
>>> forever and the relation never recovers on its own.
>>>
>>> The second attached patch makes autovacuum force an analyze when
>>> reltuples is not finite, so a corrupted value heals itself on the next
>>> autovacuum cycle. It includes a TAP test that plants an infinite
>>> reltuples and checks that autoanalyze restores a finite value.
>>>
>>
>> I'm skeptical about this change. Yeah, it'd probably help in this
>> particular Infinity case, but it can be bogus in many other ways. We
>> should really be careful to not put bogus values into reltuples.
>>
>
> I experimented with this a bit more, but I'm rather skeptical.
>
> I understand the desire to self-heal, but I don't think we do that
> elsewhere. We assume the data is correct. I'm sure we had a bunch of
> data corruption bugs, and I don't quite see why this particular case
> would be any different.
>
> I don't think Infinity/NaN values are somewhat special - sure, they're
> defined as special, but those are two singular values. If the stack has
> a random value, what's the chance it's Infinity/Nan?
>
> Isn't it more likely it'll be a valid float value? And the consequences
> will be almost exactly the same - the threshold may be so high it'll
> never be hit, right? But the isfinite() check won't catch that.
>
> So I think we'd need to check the tuple density. The 0002 patch does
> that, and it compares it to MaxHeapTuplesPerPage - it we got a higher
> density, something has to be wrong and analyze is forced.
>
> But MaxHeapTuplesPerPage is still way higher than normal densities. It's
> ~290 for 8K pages, and regular densities may be ~50, so an order of
> magnitude less. So we still need to mitigate these cases, most likely by
> suggesting a manual ANALYZE on some tables in the release notes. Maybe
> we should just do that for all cases?
>
> Another possible argument just occurred to me - one of the statistics
> import use cases is to allow messing with these fields, to import stats
> to simulate a much larger database, etc. Wouldn't this self-healing
> potentially interfere with that?
>
Oh, one more thing - I'm trying how to figure out how widespread this
issue could be, which I guess could affect how far we want to go.

I did a lot of experiments, and I only ever saw the field initialized to
0 (so it produced the right result). Maybe I'm just lucky, but more
likely it depends on platform / kernel version / compiler? What do I
need to do to actually see a garbage value?

In fact, not even valgrind reports the access as invalid. I guess that's
one reason why I missed the issue when working on the feature :-/

regards

--
Tomas Vondra