| From: | Tomas Vondra <tomas(at)vondra(dot)me> |
|---|---|
| To: | Jan Nidzwetzki <jan(at)planetscale(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: pg_class.reltuples can become non-finite and never recovers |
| Date: | 2026-07-20 13:02:14 |
| Message-ID: | 51a25e92-c3af-4172-bfda-d0fe311686f2@vondra.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
>
> 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.>
> 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.
>
> 4. Statistics import accepts Infinity and NaN
> ---------------------------------------------
>
> Finally, the reltuples argument of pg_restore_relation_stats() (and the
> shared relation_statistics_update_internal() path) is only validated
> with:
>
> if (reltuples < -1.0) /* relation_stats.c:126 */
> {
> ereport(WARNING, ...);
> result = false; /* update skipped */
> }
>
> Both Infinity and NaN pass this check and are stored verbatim, while
> an ordinary out-of-range value like -5 is correctly rejected:
>
> -+-
> CREATE TABLE t();
>
> -- returns t, reltuples := Infinity
> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
> 'version',190000,'reltuples','Infinity'::real);
>
> -- returns t, reltuples := NaN
> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
> 'version',190000,'reltuples','NaN'::real);
>
> -- WARNING, returns f
> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
> 'version',190000,'reltuples','-5'::real);
> -+-
>
> Since pg_dump --statistics / pg_upgrade round-trip reltuples, a
> corrupted value (from any of the above) also propagates across
> dump/restore and major-version upgrades.
>
> We have not addressed this one in the attached patches, because
> rejecting non-finite values here is a slightly larger policy question
> (error vs. clamp-to -1). If there is agreement on the desired
> behavior, we are glad to propose a patch for this as well.
>
OTOH this seems like something we might want to do, to validate and
reject clearly bogus values.
regards
--
Tomas Vondra
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-07-20 13:16:13 | Re: log_postmaster_stats |
| Previous Message | Fujii Masao | 2026-07-20 12:44:05 | Re: [PATCH] Don't call ereport(ERROR) from recovery target GUC assign hooks |