| From: | Sebastian Webber <sebastian(at)swebber(dot)me> |
|---|---|
| To: | Manu <manuelreyesbravo(at)gmail(dot)com> |
| Cc: | pgsql-bugs(at)lists(dot)postgresql(dot)org, Staroverov Ilja <i(dot)staroverov(at)ftdata(dot)ru> |
| Subject: | Re: VACUUM/ANALYZE double-count the backend's own pending table stats |
| Date: | 2026-10-14 20:28:17 |
| Message-ID: | CACV2tSw-_ZxCUK-8MLrgb1jz9adO_B6fyQ28VTiJAhGJH=iHgg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hi Manu, Ilya,
On Wed, 14 Oct 2026 at 15:47, Manu <manuelreyesbravo(at)gmail(dot)com> wrote:
> The attached delta sits on top of yours, untouched; it applies clean
> (+18/-8) and closes it
Confirmed here. master at d733fcaf5ca, meson debug build with cassert,
Debian trixie. v1 alone and v1 + your delta both apply, build without
new warnings and pass the full suite (meson test). The stats test
passes with each patch and fails on unpatched master.
I also measured what the double count costs autovacuum. The attached
script needs only psql and a scratch server with
autovacuum_naptime=1s. It runs three workloads on 20 tables each,
with each cycle in its own short session: insert + ANALYZE,
delete + VACUUM and insert + VACUUM. Each workload has a control that
lets the pending stats flush before the ANALYZE or VACUUM. The table
shows extra autoanalyze runs for the first workload and extra
autovacuum runs for the other two, per 20 tables:
control unpatched v1 v1 + delta
insert + ANALYZE 0 20 0 0
delete + VACUUM 0 20 0 0
insert + VACUUM 0 20 20 0
The unpatched column is the same on the official 14.24, 15.19, 16.15,
17.11 and 18.6 images. 14 still has the stats collector, so this
predates shared-memory stats. The controls stay at 0 on every build.
On whether this is just an approximation: 81e82a2bd48 fixed the same
class of problem in 2016 (deltas of an earlier transaction in the
same backend not sent yet, for TRUNCATE in a prepared transaction)
and called it an evident oversight.
One thing about the test, from the same commit message: a regression
test for d42358efb was reverted because it was too timing-sensitive
for the buildfarm. The new test passes with the fix whatever the
flush timing, so it should not be flaky. But if a flush slips in
between statements on a slow animal, it also passes without the fix,
so it may not catch a regression there.
Disclosure: I used Claude Opus 5.5 (reasoning high) for the analysis
and tests. Codex gpt-6.1-sol (reasoning high) checked independently
whether this could be by design. I reviewed the results.
Regards,
Sebastian
Em qua., 14 de out. de 2026 às 15:47, Manu <manuelreyesbravo(at)gmail(dot)com>
escreveu:
> Hi Ilya,
>
> I reproduced v1 and stress-tested it: it holds up. 8000 randomized
> single-session sequences (lazy vs eager flush as the oracle), plus
> 2PC, the truncdropped paths and post-snapshot inserts all match
> count(*) with the patch. Nice work.
>
> One thing, which is the corner you already called out:
>
> > 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.
>
> It is the same double count your patch fixes for live/dead, in the same
> function, and it does fire a spurious insert-triggered autovacuum right
> after a manual VACUUM. Minimal:
>
> INSERT 1000; VACUUM t; SELECT pg_stat_force_next_flush();
> -- n_ins_since_vacuum = 1000, should be 0
>
> The attached delta sits on top of yours, untouched; it applies clean
> (+18/-8) and closes it: a pending ins_before_vacuum field, appended at
> the end of the struct so the inline pgstat_count_* offsets do not move
> (keeps a back-branch fix viable). pgstat_report_vacuum() records what it
> already covered; the flush adds only the difference, so tuples_inserted
> and n_tup_ins stay cumulative. make check is green and the oracle drops
> to zero failures (minus TRUNCATE, below). Fold it into v2 if you like,
> or I can post it as a separate 0002.
>
> Two pre-existing things the oracle flagged that predate your patch (same
> on unpatched master, so not v1's doing): pending inserts before a
> TRUNCATE, and n_dead_tup after a subxact rolled back past a TRUNCATE.
> Both look like they need the truncdrop save/restore across
> subtransactions. Happy to send the repros separately if useful.
>
> Regards,
> Manu
>
--
Sebastian Webber
| Attachment | Content-Type | Size |
|---|---|---|
| pending-stats-autovacuum.sh | text/x-shellscript | 5.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-10-14 22:53:18 | Re: VACUUM/ANALYZE double-count the backend's own pending table stats |
| Previous Message | Manu | 2026-10-14 18:47:04 | Re: VACUUM/ANALYZE double-count the backend's own pending table stats |