Re: Fix cumulative WAL statistics double-counting from parallel workers

>
>
>
> Turns out there is a current bug in master in how pg_stat_wal and
> pg_stat_get_backend_wal() report activity from parallel workers. This
> issue has existed since the introduction of pg_stat_wal in Postgres
> 14: We double count WAL produced in parallel workers, because we add
> WAL activity from the parallel workers to both the worker's pgWalUsage
> (direct) and the leader's pgWalUsage (via InstrAccumParallelQuery),
> and each of them emits that activity to the cumulative stats.
>

Nice Catch!

> Here is a reproducer for parallel index vacuuming, but the same issue
> applies to WAL activity from pruning, or hint bits set during parallel
> sequential scans:
>
> SET max_parallel_maintenance_workers = 4;
> SET min_parallel_index_scan_size = 0;
> SET maintenance_work_mem = '256MB';
>
> CREATE TABLE t (id int, a int, b int, c int, d int) WITH
> (autovacuum_enabled = off);
> INSERT INTO t SELECT g, g, g, g, g FROM generate_series(1, 2000000) g;
> CREATE INDEX ON t (a);
> CREATE INDEX ON t (b);
> CREATE INDEX ON t (c);
> CREATE INDEX ON t (d);
> DELETE FROM t WHERE id % 2 = 0;
> CHECKPOINT;
>
> SELECT pg_stat_force_next_flush();
> SELECT pg_current_wal_lsn() AS lsn_before,
> (SELECT wal_bytes FROM pg_stat_wal) AS reported_before \gset
>
> VACUUM (PARALLEL 4) t;
>
> SELECT pg_stat_force_next_flush();
> SELECT pg_current_wal_lsn() AS lsn_after,
> (SELECT wal_bytes FROM pg_stat_wal) AS reported_after \gset
>
> SELECT pg_wal_lsn_diff(:'lsn_after', :'lsn_before') AS actual_wal_bytes,
> :reported_after - :reported_before AS reported_wal_bytes,
> round((:reported_after - :reported_before) * 100.0
> / pg_wal_lsn_diff(:'lsn_after', :'lsn_before'), 1) AS
> reported_pct_of_actual;
>
> On master this returns:
>
> -[ RECORD 1 ]----------+----------
> actual_wal_bytes | 195781792
> reported_wal_bytes | 257020498
> reported_pct_of_actual | 131.3
>
> ---
>
> In terms of how to fix this, I first tried to invent a mechanism that
> would keep instrumentation imported from workers separate in the
> leader (so we never double count in the global). But that turns out to
> be pretty tricky, because we have use cases (pg_stat_statements being
> the main one, but e.g. also EXPLAIN ANALYZE of a query calling a
> function that itself runs a parallel query) where we want to track
> whole-statement activity (leader + workers), including utility
> statements, and we want to be able to do so in a nested manner.
>
> Instead, at least for now, the easier solution seems to be to
> accumulate imported instrumentation data into both the existing
> pg(Wal|Buffer)Usage globals, and a separate set called
> pg(Wal|Buffer)UsageFromWorkers that counts only the activity from the
> workers. Any caller that wants to isolate the leader-only activity for
> reporting to the cumulative statistics system then subtracts the
> worker portion from the global instrumentation using a helper function
> (ComputeOwnWalUsage), and workers continue to report their own
> activity directly to the cumulative statistics system.
>
> See attached.
>
> This same approach will also be useful to replace the use of
> pgStatBlock(Read|Write)Time with diffing pgBufferUsage instead for the
> cumulative reporting in pg_stat_database, as discussed in [0]. The
> stack-based instrumentation proposal [1] will have to follow this
> change of having a separate global to remember what came from the
> workers, but that's pretty straightforward to update.
>
> Thoughts?
>
> Thanks,
> Lukas
>
> [0]:
> https://www.postgresql.org/message-id/flat/605732d2-c6bd-4c65-ac83-8d071935d860%40gmx.at
> [1]:
> https://www.postgresql.org/message-id/flat/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg%40mail.gmail.com
>
> --
> Lukas Fittl
>