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

From: Lukas Fittl <lukas(at)fittl(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Ahmed Gouda <gouda0x(at)gmail(dot)com>, Bernd Reiß <bd_reiss(at)gmx(dot)at>
Subject: Re: Fix cumulative WAL statistics double-counting from parallel workers
Date: 2026-11-03 22:58:31
Message-ID: CAP53PkzFJwmHuKSYf0Yga81UmpHqHsuFBLgEwTCP-1H1ZJhduQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael,

Thanks for taking a look!

On Mon, Nov 2, 2026 at 1:32 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> 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.

The problem isn't just out of core extensions using pgWalUsage, its
that we also look at pgWalUsage for reporting in EXPLAIN and on
statements that ran parallel that were called by a node, as mentioned
in my initial email.

Here is an example:

CREATE TABLE t (a int) WITH (autovacuum_enabled = off);
INSERT INTO t SELECT generate_series(1, 1000000);
CHECKPOINT; -- so that setting hint bits (checksums on) emits FPIs

CREATE FUNCTION f() RETURNS bigint LANGUAGE plpgsql AS
$$ DECLARE n bigint; BEGIN SELECT count(*) INTO n FROM t; RETURN n; END $$;

SET parallel_setup_cost = 0;
SET parallel_tuple_cost = 0;
SET max_parallel_workers_per_gather = 2;
SET parallel_leader_participation = off; -- all WAL comes from the workers

EXPLAIN (ANALYZE, WAL, BUFFERS OFF, COSTS OFF, TIMING OFF, SUMMARY
OFF) SELECT f();

QUERY PLAN
----------------------------------------------------------------------------------
Result (actual rows=1.00 loops=1)
WAL: records=8849 fpi=4426 bytes=36596662 fpi bytes=36114392
buffers full=3975
(2 rows)

Now, taking that example, here are three possible approaches to the
problem overall that I can see, focusing on what to do on master (i.e.
ignoring extension compatibility concerns):

1) pgWalUsage keeps today's meaning, pgWalUsageFromWorkers is
additionally accumulated in InstrAccumParallelQuery (the v1 patch)

- When the function finishes executing, the worker instrumentation
gets fed into the parent process pgWalUsage and pgWalUsageFromWorkers
- That pgWalUsage is then seen by the Result node as part of its
InstrStartNode/InstrStopNode, which results in the correct reporting
- Use cases like pg_stat_statements that want to report worker
activity as part of the statement only look at pgWalUsage
- The pg_stat_wal reporting in the leader subtracts
pgWalUsageFromWorkers with WalUsageAccumDiff so it correctly skips the
data from this invocation

2) pgWalUsage does not include the worker instrumentation, maybe
rename to pgWalUsageOwn (basically your suggestion for master, if I
understood correctly), we also introduce pgWalUsageFromWorkers

- When the function finishes executing, the worker instrumentation
only gets fed into pgWalUsageFromWorkers
- In order for the Result node to see the activity, it needs to diff
both pgWalUsage and pgWalUsageFromWorkers in InstrStopNode, to
correctly increment its instrumentation data to include workers
- Use cases like pg_stat_statements need to look at both pgWalUsage
and pgWalUsageFromWorkers
- The pg_stat_wal reporting in the leader can simply work with
pgWalUsage like today

3) pgWalUsage keeps today's meaning, we introduce a separate
pgWalUsageOwn that includes only the leader

- pgWalUsageOwn gets additionally incremented in all places where we
currently increment pgWalUsage
- When the function finishes executing, the worker instrumentation
only gets fed into pgWalUsage
- That pgWalUsage is then seen by the Result node as part of its
InstrStartNode/InstrStopNode, which results in the correct reporting
- Use cases like pg_stat_statements that want to report worker
activity as part of the statement only look at pgWalUsage
- The pg_stat_wal reporting in the leader only looks at the new pgWalUsageOwn

I don't think (2) is workable, especially if we want a design that
also works for pgBufferUsage - doing any extra work in InstrStopNode
is too expensive (instruction-wise, and because of the size of the
memory being accessed).

I think (3) could potentially be an alternate, but I don't know if its
worth the trouble just to avoid the WalUsageAccumDiff on flush, which
is not in the hot path.

Did I miss something in how you think this could work?

Thanks,
Lukas

--
Lukas Fittl

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-11-03 23:07:33 Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash
Previous Message Chao Li 2026-11-03 22:20:59 Re: Copy-pasto in comment in ExecAppendAsyncReset