Fix cumulative WAL statistics double-counting from parallel workers
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254450psql -h localhost -U postgresBuilt from patchset v1 (message #1), November 01, 2026 at 11:12 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254450_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254450_1 && git checkout t254450_1Patchset v1 (message #1) is on t254450_1
Hi,
Over in [0]/messages/by-id/605732d2-c6bd-4c65-ac83-8d071935d860@gmx.at we were recently discussing how parallel workers
accumulate their buffer usage into the leader and how that makes it
hard to pass pgBufferUsage instrumentation into the cumulative stats
system, and I've also been looking into this for the stack-based
instrumentation work [1]/messages/by-id/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com, because it's really hard to avoid double
counting due to how we accumulate into the leader's
pgBufferUsage/pgWalUsage today.
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.
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]/messages/by-id/605732d2-c6bd-4c65-ac83-8d071935d860@gmx.at. The
stack-based instrumentation proposal [1]/messages/by-id/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com 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]: /messages/by-id/605732d2-c6bd-4c65-ac83-8d071935d860@gmx.at
[1]: /messages/by-id/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com
--
Lukas Fittl
Hi,
On Thu, Oct 29, 2026 at 03:58:03PM -0700, Lukas Fittl wrote:
Hi,
Over in [0] we were recently discussing how parallel workers
accumulate their buffer usage into the leader and how that makes it
hard to pass pgBufferUsage instrumentation into the cumulative stats
system, and I've also been looking into this for the stack-based
instrumentation work [1], because it's really hard to avoid double
counting due to how we accumulate into the leader's
pgBufferUsage/pgWalUsage today.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.
Ouch, nice catch!
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.
Agreed.
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.
Yeah that looks like the best way to proceed currently.
I wonder if the extra additions could be reduced though. What about first
accumulating all workers into a local BufferUsage/WalUsage and then calling
InstrAccumParallelQuery() once for all workers?
Also, worth to add some tests?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi Bertrand,
On Fri, Oct 30, 2026 at 12:27 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
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.Agreed.
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.Yeah that looks like the best way to proceed currently.
Great, thanks for thinking through it!
I wonder if the extra additions could be reduced though. What about first
accumulating all workers into a local BufferUsage/WalUsage and then calling
InstrAccumParallelQuery() once for all workers?
Hmm. Not sure if the extra additions really matter (I don't think its
a particularly performance critical code section), and I'm hesitant to
have more duplicated code in the different call sites of
InstrAccumParallelQuery.
We could refactor it a bit by having InstrAccumParallelQuery take
arrays of WalUsage/BufferUsage, as well as a worker count argument,
and then save on the additions that way inside
InstrAccumParallelQuery?
We have this pattern of working on multiple instrumentation structs
already in e.g. InstrAllocTrigger, so it doesn't seem too far fetched
to do - and all callers of InstrAccumParallelQuery have a (currently
duplicated) loop over the workers.
Also, worth to add some tests?
Maybe? I'm always on the fence with how much tests we should be adding
for instrumentation fixes, because it needs a bunch of setup, can be
brittle in terms of comparing numbers (especially since producing a
certain amount of WAL is involved), and more tests slow down the
regression test suite. But happy to add it if we feel its worth it.
FWIW, what I'd really wish for is a better way to guarantee the
invariant (i.e. nobody starts diffing pgWalUsage/pgBufferUsage and
emitting it into cumulative stats without taking care of this), but I
feel the only way to achieve that would be to make the globals private
to instrument.c and require the use of an appropriately named method
to access them, which doesn't seem worth it.
Writing this out though makes me wonder though if we should add a
comment above pgBufferUsage/pgWalUsage in instrument.h warning of this
hazard, instead of just on the new globals below.
Thanks,
Lukas
--
Lukas Fittl
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!
Show quoted text
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 \gsetVACUUM (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 \gsetSELECT 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]:
/messages/by-id/605732d2-c6bd-4c65-ac83-8d071935d860@gmx.at
[1]:
/messages/by-id/CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com--
Lukas Fittl
Hi Lukas,
First, sorry for the previous incomplete email, sent by accident; please
ignore it.
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!
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.
I was thinking: instead of introducing new pg(Wal|Buffer)UsageFromWorkers
global variables, could we make the existing `static WalUsage prevWalUsage`
a global variable, rename it to reportedWalUsage, and increment it directly
in InstrAccumParallelQuery() and on flush subtract it from pgWalUsage? That
would also minimize the number of subtraction operations on flush.
And we can do the same with pgStatBlock(Read|Write)Time, rename it to
reportedBlock(Read|Write)Time, etc…
Regards,
On Thu, Oct 29, 2026 at 03:58:03PM -0700, Lukas Fittl wrote:
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.
If I am reading the patch right, it means that a leader's pgWalUsage
includes both its own activity *and* the activity of its workers. It
sounds to me like a mistake to do that. Why aren't pgWalUsage and
friends handled in such a way that they only include the activity of
their own process? That would be a lot cheaper, especially for the
pgstats parts.
+void
+ComputeOwnWalUsage(WalUsage *walusage)
+{
+ memset(walusage, 0, sizeof(WalUsage));
+ WalUsageAccumDiff(walusage, &pgWalUsage, &pgWalUsageFromWorkers);Yeah, so I feel that I am reading that right, then. When we want to
get the WAL usage from the leader, before aggregating it back to
pgstats, we substract the data that came from the workers. That seems
like a waste, we're doing diffs calculations because we gather all the
data in one single place. But all that should just be about
additions. You are making it additions then subtractions, leaving the
places where we want aggregates untouched.
You will need the data from the workers, but I'd suggest to do that
the other way around: gather the data from the workers in a secondary
WalUsage, apply the sum of all only when you want to have a view of
the aggregate. That should sum up to:
heap_vacuum_rel()
do_analyze_rel()
PGSS
instrument.c
And there should be no need to touch at the pgstats code paths.
--
Michael
Hi Lukas,
On Fri, Oct 30, 2026 at 12:19:51PM -0700, Lukas Fittl wrote:
On Fri, Oct 30, 2026 at 12:27 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:We could refactor it a bit by having InstrAccumParallelQuery take
arrays of WalUsage/BufferUsage, as well as a worker count argument,
and then save on the additions that way inside
InstrAccumParallelQuery?
Yeah that makes sense. That said, as this should be backpatched, we'd either have
to change the function signature in the back branches or make master diverge
from them. Neither option looks appealing.
I'm not sure either is worth it given this is not performance critical though,
so I'd vote to keep v1 as is in all branches.
Also, worth to add some tests?
Maybe? I'm always on the fence with how much tests we should be adding
for instrumentation fixes, because it needs a bunch of setup, can be
brittle in terms of comparing numbers (e
Right, and a test relying on workers actually processing enough WAL work might
also be fragile in the buildfarm. So I would not insist on adding one here.
FWIW, what I'd really wish for is a better way to guarantee the
invariant (i.e. nobody starts diffing pgWalUsage/pgBufferUsage and
emitting it into cumulative stats without taking care of this), but I
feel the only way to achieve that would be to make the globals private
to instrument.c and require the use of an appropriately named method
to access them, which doesn't seem worth it.
Agreed.
Writing this out though makes me wonder though if we should add a
comment above pgBufferUsage/pgWalUsage in instrument.h warning of this
hazard, instead of just on the new globals below.
+1, I think that should be enough for now.
One comment about the patch.
=== 1
+extern PGDLLIMPORT BufferUsage pgBufferUsageFromWorkers;
+extern PGDLLIMPORT WalUsage pgWalUsageFromWorkers;I wonder if those shouldn't remain private to instrument.c? Contrary to pgWalUsage,
no callers need to access them directly, and they look more like internal such as
save_pgWalUsage. So maybe they could be static and only ComputeOwnWalUsage()
exposed?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi,
On Mon, Nov 02, 2026 at 05:16:29PM +0900, Michael Paquier wrote:
On Thu, Oct 29, 2026 at 03:58:03PM -0700, Lukas Fittl wrote:
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.If I am reading the patch right, it means that a leader's pgWalUsage
includes both its own activity *and* the activity of its workers. It
sounds to me like a mistake to do that. Why aren't pgWalUsage and
friends handled in such a way that they only include the activity of
their own process?
Wouldn't that change the semantics of pgWalUsage for out of core extensions using
it (if any) the same way as PGSS? We can adjust PGSS, but those extensions would
start seeing leader only activity once running a version with this change?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Mon, Nov 02, 2026 at 08:41:15AM +0000, Bertrand Drouvot wrote:
On Mon, Nov 02, 2026 at 05:16:29PM +0900, Michael Paquier wrote:
If I am reading the patch right, it means that a leader's pgWalUsage
includes both its own activity *and* the activity of its workers. It
sounds to me like a mistake to do that. Why aren't pgWalUsage and
friends handled in such a way that they only include the activity of
their own process?Wouldn't that change the semantics of pgWalUsage for out of core extensions using
it (if any) the same way as PGSS? We can adjust PGSS, but those extensions would
start seeing leader only activity once running a version with this change?
We don't have to change the meaning of pgWalUsage in the stable
branches. We could keep it there, where I'd suggest to invent a third
WalUsage then that aggregates only the data of its own. Then, we use
the "own" WalUsage for the stats, and for the combined view use the
"own" WalUsage and pgWalUsageFromWorkers. Finally, remove all traces
of pgWalUsage from the backend code, remove its definition on HEAD.
Perhaps the last point is not required.
My point is that I don't get the meaning of these new
WalUsageAccumDiff(), especially as we could design things so as we
avoid them.
--
Michael