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

On Tue, Nov 3, 2026 at 3:18 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Tue, Nov 03, 2026 at 02:58:31PM -0800, Lukas Fittl wrote:
> > 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).
>
> Hmm. I have not looked at that. But would it really matter if we
> just drop pgWalUsage? We only need the aggregated data from the
> leaders and its workers when we are ready to report the data.

When you say "ready to report the data", which moment are you thinking of?

The problem in the EXPLAIN example shown is that we need some way to
have the instrumentation for the Result node capture the function's
query parallel worker activity. Currently this works because we're
inside an open InstrStartNode / InstrStopNode pair, and the
InstrAccumParallelQuery call when the function's query ends makes the
leader pgWalUsage move forward the expected amount.

The only way to avoid extra work in InstrStopNode is to have a global
that represents what pgWalUsage currently is (leader + worker), but
maybe I'm missing something in your argument?

>
> > 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.
>
> With only HEAD in mind, (2) just seems like the simplest alternative,
> because there is less to consider about all these states except
> summing them up, where necessary. The origin of the double-counting
> comes from the point of pgWalUsage being rather confusing in itself,
> because we do not define when it is okay or when is is not okay to
> use. If it's confusing, I would not be sad to see it gone on HEAD,
> replaced by a WalUsage that just tracks the activity within the
> process, combined with a secondary WalUsage where we aggregate from
> activity from the workers.

I agree on the goal (I think its very confusing right now), but I
don't see a good way forward for (2) due to the InstrStopNode overhead
problem.

What we could theoretically do is a variant of (3), where we rename
the current pgWalUsage to pgWalUsageWithWorkers, and introduce a new
pgWalUsage that doesn't factor in the workers. But we'd still have the
extra instructions in all the places where we increment the activity.

Thanks,
Lukas

--
Lukas Fittl