From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: per backend WAL statistics |
Date: | 2025-03-06 10:33:52 |
Message-ID: | Z8l6EMM4ImVoWRkg@ip-10-97-1-34.eu-west-3.compute.internal |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On Wed, Mar 05, 2025 at 05:26:40PM +0000, Bertrand Drouvot wrote:
> So yeah, back to the issue, we have to pay more attention for the WAL stats
> because pgWalUsage is "incremented" without any check with pgstat_tracks_backend_bktype()
> (that's not the case for the IO stats where the counters are incremented taking
> into account pgstat_tracks_backend_bktype()).
>
> So for the moment, in the attached I "just" add a pgstat_tracks_backend_bktype()
> check in pgstat_backend_have_pending_cb() but I'm not sure I like it that much...
>
> Will think more about it...
After giving more thoughts, I think that the way it's currently done makes sense.
There is no need to check pgstat_tracks_backend_bktype() while incrementing
pgWalUsage or to create a "BackendpgWalUsage" (that would be incremented based
on the pgstat_tracks_backend_bktype()).
In pgstat_create_backend() (called based on pgstat_tracks_backend_bktype()):
- PendingBackendStats is initialized to zero
- prevBackendWalUsage is initialized to pgWalUsage
But:
1. PendingBackendStats is "incremented" during IO operations, so that it makes
sense to ensure that pgstat_tracks_backend_bktype() returns true in those functions
(i.e pgstat_count_backend_io_op_time() and pgstat_count_backend_io_op()).
2. prevBackendWalUsage is not incremented, it's just there to compute the delta
with pgWalUsage in pgstat_flush_backend_entry_wal().
pgstat_flush_backend_entry_wal() is only called from pgstat_flush_backend() that
does the pgstat_tracks_backend_bktype() before. And so is
pgstat_flush_backend_entry_io().
So, I think it's fine the way it's done here. The missing part was in
pgstat_backend_have_pending_cb() which has been fixed.
But this missing part was already there for the IO stats. It just not manifested
yet maybe because PendingBackendStats was full of zeroes by "luck".
Indeed, there is no reason for pgstat_backend_have_pending_cb() to return true if
pgstat_tracks_backend_bktype() is not satisfied.
So it deserves a dedicated patch to fix this already existing issue: 0001 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v16-0001-Add-an-extra-check-in-pgstat_backend_have_pendin.patch | text/x-diff | 1.2 KB |
v16-0002-per-backend-WAL-statistics.patch | text/x-diff | 13.5 KB |
From | Date | Subject | |
---|---|---|---|
Previous Message | Amit Kapila | 2025-03-06 10:03:44 | Re: Selectively invalidate caches in pgoutput module |