Re: VACUUM/ANALYZE double-count the backend's own pending table stats

Hi Ilya,

Thank you for the patch. I tried v2 (0001 and 0002 together, the cfbot
branch cf/7460, CF entry 7460) and would like to share what I saw. I
can send the scripts and the output if they are useful.

Setup
- Master at c1694b7 (11 Nov 2026) plus the two patches as one diff (4
files: pgstat_relation.c, pgstat.h, and the stats test files). `git
apply --check` was clean on that master.
- configure with --enable-cassert --enable-debug --enable-tap-tests
--without-icu, on Linux x86_64. I built the unpatched tree first, then
the patched tree. Both builds had 0 compiler warnings.
- A small note for others: my first patched build was incremental
without --enable-depend. Since pgstat.h changes a struct, I got stale
objects, "write past chunk end in PgStat Pending" and 27 regression
failures. After `make clean` and a full rebuild, none of this
happened, so I think it was my build and not the patch.

Tests that passed
- `make check` with the patch: all 243 tests passed, including stats.

Manual tests (temporary cluster, autovacuum off for the first runs; I
compared the unpatched and the patched build, and used
pg_stat_force_next_flush() before reading pg_stat_user_tables)
- INSERT 1000, then ANALYZE in the same session (autocommit):
unpatched gives n_live_tup 2000 and n_mod_since_analyze 1000; patched
gives 1000 and 0 (count(*) is 1000).
- INSERT 1000, then VACUUM: unpatched gives n_live_tup 2000 and
n_ins_since_vacuum 1000; patched gives n_live_tup 1000 and
n_ins_since_vacuum 0.
- INSERT 1000, DELETE 400, UPDATE 100 (separate transactions), then
VACUUM (ANALYZE): unpatched gives n_live_tup 1200 and n_dead_tup 500;
patched gives 600 and 0 (count(*) is 600). n_tup_ins/upd/del stay
1000/100/400 on both builds.
- INSERT 1000 and DELETE 400 in one transaction, then VACUUM
(ANALYZE): unpatched gives n_live_tup 1200 and n_dead_tup 400; patched
gives 600 and 0.
- TRUNCATE, then INSERT 300 in a later transaction, no VACUUM or
ANALYZE: unpatched n_live_tup is 1300; patched is 300 (count(*) is
300).
- pg_stat_reset() between the INSERT and the ANALYZE: patched gives
n_live_tup 1000 (correct). pg_stat_reset() after the ANALYZE and then
50 more rows: patched gives n_live_tup 50 and n_mod_since_analyze 50,
unpatched gives 1050 for both.
- Partitioned table with two leaves, INSERT 1000, ANALYZE of the
parent: patched gives n_live_tup 499 and 501 for the leaves (the
parent shows 0), n_mod_since_analyze 0; unpatched gave 998 and 1002
with n_mod_since_analyze 499 and 501.
- TRUNCATE inside a transaction with SAVEPOINT, INSERT 5, ROLLBACK TO,
COMMIT, then INSERT 300 later: patched gives n_live_tup 300 (count(*)
is 300), unpatched 1300.
- Same scripts with autovacuum on (naptime 1s), patched build only:
the numbers right after the flush were the same as above. After 5 s,
autovacuum had analyzed the subxact table once (autoanalyze_count 1,
n_mod_since_analyze 0, n_dead_tup 5).
- Autovacuum check, patched build only: a table with
autovacuum_vacuum_insert_threshold=100 and scale factor 0 (same for
analyze), INSERT 1000, manual VACUUM, flush. Right after the flush:
n_live_tup 1000, n_mod_since_analyze 1000, n_ins_since_vacuum 0. After
5 s: autovacuum_count 0 and autoanalyze_count 1. So no extra
insert-triggered autovacuum after the manual VACUUM, which is what I
expected from 0002.
- The server did not crash during these runs (assert-enabled build). I
counted TRAP/assert/PANIC lines in the server log: 0 in the three
repro/edge runs (unpatched, patched with autovacuum off, patched with
autovacuum on). I did not grep the log of the separate autovacuum
check.

Questions and comments
- After TRUNCATE and a later INSERT 300 (case 5 above), the patched
build gives n_ins_since_vacuum 300 but n_mod_since_analyze 1300. In
the subxact case it is n_ins_since_vacuum 305 and n_mod_since_analyze
1300. I do not know if n_mod_since_analyze should also restart after a
truncate. It looks the same on the unpatched build, so I assume it is
outside this patch, but I wanted to ask in case you already thought
about it. I think this overlaps the TRUNCATE/subtransaction cases Manu
raised, which you plan to handle separately.
- In the partitioned table case the n_tup_ins and n_live_tup of the
parent stay 0 on both builds. This is only an observation.
- Bertrand raised upthread that these cases may be like the one in [1]
(not treated as a bug). I do not have an opinion on that, but the
numbers above show the first VACUUM/ANALYZE case clearly (2x
n_live_tup, and n_mod_since_analyze equal to the inserted rows right
after ANALYZE). I think splitting out the TRUNCATE fix, as you
offered, is a good idea.
- I did not review the code of pgstat_report_vacuum() and
pgstat_report_analyze() in depth.

Not tested
- 0001 and 0002 separately (I only tested them together). Each
scenario ran once, with no repeats and no variation in flush timing.
- Whether the new regression tests fail without the fix (I did not run
the patched tests against the unpatched code; Sebastian reported
upthread that the v1 stats test fails on unpatched master), and the
unpatched `make check`.
- The autovacuum scenario on the unpatched build (I only ran it on the
patched build), and Sebastian's pending-stats-autovacuum.sh script.
- Two-phase commit (PREPARE TRANSACTION and COMMIT PREPARED), TOAST,
matviews, ANALYZE with a column list, database-wide VACUUM ANALYZE,
relcache invalidation between statements.
- Several backends changing the same table at the same time.
- Back-patching and branches 14 to 18.
- make check-world, the isolation and other TAP suites (only make
check was run).
- Meson build, non-assert builds, Windows, 32-bit, other compilers, ICU builds.
- Code style (pgindent), documentation, and a full code review.

[1] https://postgr.es/m/xnd433j72ixptoydkifnleke43bvql4rxrw4ohnodx6hk4wcho%40m46ab2bt33jh#07b7a951d9a983ce9432d6bf86979ff2

Thanks again, and please let me know if you want me to rerun anything
with different settings.

Best regards,
João Rito

Em qua., 11 de nov. de 2026 às 23:31, Staroverov Ilja
<i(dot)staroverov(at)ftdata(dot)ru> escreveu:
>
> Hi Bertrand,
>
> Thanks for the pointer. Yes, the VACUUM example in that thread has the
> same cause. I agree with keeping stats updates batched, but I think
> we can avoid this particular double count without changing that policy.
>
> The patch only adjusts the pending counters of the backend running
> VACUUM or ANALYZE. When it reports new live/dead estimates, it discards
> its own pending deltas from earlier transactions, rather than adding
> them on top of those estimates at the next flush. This does not require
> more frequent flushes or access to another backend's pending stats.
> The cumulative insert/update/delete counters are preserved.
>
> pgstat_report_analyze() already subtracts the current transaction's
> changes to avoid double counting at commit, and also subtracts pending
> delta_dead_tuples. This is the same local accounting problem for changes
> committed earlier by that backend. For mod_since_analyze, the patch
> also discards the pending changed_tuples when ANALYZE resets the shared
> counter. Otherwise an INSERT of N rows followed by ANALYZE can leave
> n_mod_since_analyze at N after the pending stats are flushed, despite
> there having been no subsequent changes.
>
> Other backends' unflushed deltas can still make the estimates inaccurate.
> I agree that solving that would be a different and much larger change.
>
> > So maybe only the TRUNCATE case deserves a fix?
>
> I'd still consider the local VACUUM/ANALYZE correction worthwhile. It
> avoids reintroducing changes already covered by that backend's own
> maintenance operation, including changes that can trigger another
> autoanalyze or autovacuum, while retaining the existing flush policy.
> I can separate the TRUNCATE fix so it can be reviewed independently.
>
> --
> Ilya Staroverov
>