VACUUM/ANALYZE double-count the backend's own pending table stats (n_live_tup = 2*N after INSERT + ANALYZE)

Hi,

VACUUM and ANALYZE overwrite live_tuples/dead_tuples in the shared stats
entry with their own counts. Those counts already include the effects
of transactions this backend committed earlier. But the matching deltas
can still be pending in the backend's local stats, because pending
stats are flushed at most once per PGSTAT_MIN_INTERVAL. They are then
added again on top of the new values at the next flush.

I know these counters are only approximate when other sessions modify
the table concurrently. This case is different: it happens in a single
session, it is deterministic, and the backend has exact information
about its own pending counts.

Reproduced on 18.6 and on master (8b9ffcb11), in autocommit mode:

CREATE TABLE t (id int);
INSERT INTO t SELECT generate_series(1, 1000);
ANALYZE t; -- or VACUUM t;
SELECT pg_stat_force_next_flush();
SELECT n_live_tup, n_dead_tup, n_mod_since_analyze
FROM pg_stat_user_tables WHERE relname = 't';

n_live_tup | n_dead_tup | n_mod_since_analyze
------------+------------+---------------------
2000 | 0 | 1000

With a DELETE, the result goes the other way:

-- t has 1000 rows, stats flushed
DELETE FROM t WHERE id <= 500;
ANALYZE t;
SELECT pg_stat_force_next_flush();
-- n_live_tup = 0, n_dead_tup = 500 (count(*) = 500)

-- same, but VACUUM instead of ANALYZE:
-- n_live_tup = 0, n_dead_tup = 500 (the dead tuples were removed)

pg_class.reltuples is correct in all these cases. Only the cumulative
stats are wrong, and they stay wrong until the next VACUUM/ANALYZE.
When the INSERT and ANALYZE run in a single explicit transaction, the
result is correct, because pgstat_report_analyze() subtracts the counts
of the in-progress transaction (rel->pgstat_info->trans) precisely to
avoid double counting. It doesn't do the same for already-committed but
still-pending deltas. It does subtract the pending delta_dead_tuples
("count stuff inserted by already-aborted subxacts, too"), but not
delta_live_tuples. pgstat_report_vacuum() doesn't look at pending stats
at all.

Why it matters: pg_class.reltuples is fine, but the counters that
autovacuum actually uses are inflated:
- After INSERT + ANALYZE, mod_since_analyze is N right after the
ANALYZE, which brings the next autoanalyze forward.
- After DELETE + VACUUM, dead_tuples is reported as N although
VACUUM has just removed them, which can trigger an extra
autovacuum.
n_live_tup is also what monitoring and bloat estimation queries
usually read. The pattern "bulk load / bulk delete, then ANALYZE or
VACUUM in the same session" is common in ETL scripts.

A related problem is in the same file. AtEOXact_PgStat_Relations()
does

relstat->tab.truncdropped = trans->truncdropped;

on every commit. So if one transaction truncates a table and a later
transaction of the same backend modifies it before the pending stats
are flushed, the pending truncation reset is lost. No VACUUM or ANALYZE
is needed for this one:

-- t has 1000 rows, stats flushed
TRUNCATE t;
INSERT INTO t SELECT generate_series(1, 300);
SELECT pg_stat_force_next_flush();
-- n_live_tup = 1300 (count(*) = 300)

pgstat_twophase_postcommit() does the same.

The attached patch for master:
- Makes VACUUM and ANALYZE forget the pending delta_live_tuples and
delta_dead_tuples of the relation, because their new totals
supersede them. With ANALYZE's resetcounter, it also forgets the
pending changed_tuples. A pending truncation reset is superseded as
well. For ANALYZE, its reset of ins_since_vacuum is applied right
away. The old subtraction of pending delta_dead_tuples in ANALYZE
goes away, because the new code covers it. Forgetting the deltas
is better than subtracting them, because subtracting interacts
badly with the clamping to 0. For example, after DELETE + VACUUM,
dead_tuples would be clamped to 0 and then the pending deltas
would be added back anyway.
- Only ever sets truncdropped at commit and COMMIT PREPARED, never
clears it.
- Adds a regression test to stats.sql. The test fails without the
fix (n_live_tup 12/13/20 instead of 6/3/10). It passes with the
fix whether or not the stats happen to get flushed between
statements.

One thing this patch doesn't fix: after INSERT + VACUUM in the same
session, ins_since_vacuum is still N, because the flush adds the
pending tuples_inserted to it. That counter also feeds n_tup_ins, so
fixing it would need a separate field. I left it alone.

The patch conflicts with the "pgstat: Flush some statistics within
running transactions, take 2" patch (v17). That patch doesn't change
either behavior, but with its flushed_xact baselines, "forget the
pending deltas" would presumably have to mean "advance the baseline"
rather than "zero the counters". I can rebase in whichever order is preferred.

--
Ilya Staroverov