From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: per backend WAL statistics |
Date: | 2025-01-21 07:19:55 |
Message-ID: | Z49KmyAIFQtDp0bV@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 Fri, Jan 17, 2025 at 08:43:57AM +0900, Michael Paquier wrote:
> On Thu, Jan 16, 2025 at 12:44:20PM -0500, Andres Freund wrote:
> > On 2025-01-16 17:11:09 +0000, Bertrand Drouvot wrote:
> >> So, do you think that the initial proposal that has been made here (See R1. in
> >> [2]) i.e make use of a new PendingBackendWalStats variable:
> >
> > Well, I think this first needs be fixed for for the IO stats change made in
> >
> > Once we have a pattern to model after, we can apply the same scheme here.
>
> Okay, thanks for the input. I was not sure what you intended
> originally with all this part of the backend code, and how much would
> be acceptable. The line is clear now.
>
> >> 0003 does not rely on pgstat_prep_backend_pending() for its pending statistics
> >> but on a new PendingBackendWalStats variable. The reason is that the pending wal
> >> statistics are incremented in a critical section (see XLogWrite(), and so
> >> a call to pgstat_prep_pending_entry() could trigger a failed assertion:
> >> MemoryContextAllocZero()->"CritSectionCount == 0 || (context)->allowInCritSection"
> >> "
> >>
> >> and implemented up to v4 is a viable approach?
> >
> > Yes-ish. I think it would be better to make it slightly more general than
> > that, handling this for all types of backend stats, not just for WAL.
>
> Agreed to use the same concept for all these parts of the backend
> stats kind rather than two of them. Will send a reply on the original
> backend I/O thread as well.
PFA v6 that now relies on the new PendingBackendStats variable introduced in
4feba03d8b9.
Remark: I moved PendingBackendStats back to pgstat.h because I think that the
"simple" pending stats increment that we are adding in xlog.c are not worth
an extra function call overhead (while it made more sense for the more complex IO
stats handling). So PendingBackendStats is now visible to the outside world like
PendingWalStats and friends.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v6-0001-Extract-logic-filling-pg_stat_get_wal-s-tuple-int.patch | text/x-diff | 3.9 KB |
v6-0002-Adding-a-new-PgStat_WalCounters-struct.patch | text/x-diff | 5.0 KB |
v6-0003-per-backend-WAL-statistics.patch | text/x-diff | 18.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Logan MAUZAIZE | 2025-01-21 07:25:18 | Re: Proper way to clean-up connection for reuse (`DISCARD ALL` and default role) |
Previous Message | Hayato Kuroda (Fujitsu) | 2025-01-21 07:03:05 | pgbench without dbname worked differently with psql and pg_dump |