| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Lukas Fittl <lukas(at)fittl(dot)com> |
| Cc: | 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-02 08:16:29 |
| Message-ID: | auhG3fVhiQ1cmfGo@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-11-02 08:33:19 | Re: Fix cumulative WAL statistics double-counting from parallel workers |
| Previous Message | Chao Li | 2026-11-02 08:10:30 | Re: [PATCH] Combine qual-based and NOT NULL proofs when reducing outer joins |