public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: Andres Freund <[email protected]>
Subject: Re: per backend I/O statistics
Date: Wed, 15 Jan 2025 18:27:22 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAN55FZ0ZQTf-D+5Rup4H6zp+n3E2EuonYuzTiPkp=1GhesZU_A@mail.gmail.com>
	<[email protected]>

On Wed, Jan 15, 2025 at 08:30:20AM +0000, Bertrand Drouvot wrote:
> On Wed, Jan 15, 2025 at 11:03:54AM +0300, Nazir Bilal Yavuz wrote:
>> With this commit it may not be possible to count IOs in the critical
>> sections. I think the problem happens only if the local
>> PgStat_BackendPending entry is being created for the first time for
>> this backend in the critical section.
> 
> Yeah, I encountered the exact same thing and mentioned it in [1] (see R1.).
> 
> In [1] I did propose to use a new PendingBackendWalStats variable to "bypass" the
> pgstat_prep_backend_pending() usage.
> 
> Michael mentioned in [2] that is not really consistent with the rest (what I
> agree with) and that "we should rethink a bit the way pending entries are
> retrieved". I did not think about it yet but that might be the way to
> go, thoughts? 
> 
> [1]: https://www.postgresql.org/message-id/Z3zqc4o09dM/Ezyz%40ip-10-97-1-34.eu-west-3.compute.internal
> [2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz

My problem is that this is not only related to backend stats, but to
all variable-numbered stats kinds that require this behavior.  One
other case where I've seen this as being an issue is injection point
stats, for example, while enabling these stats for all callbacks and
some of them are run in critical sections.

A generic solution to the problem would be to allow
pgStatPendingContext to have MemoryContextAllowInCriticalSection() set
temporarily for the allocation of the pending data.
Perhaps not for all stats kinds, just for these where we're OK with
the potential memory footprint so we could have a flag in
PgStat_KindInfo.  Giving to all stats kinds the responsibility to
allocate a pending entry beforehand outside a critical section is
another option, but that means going through different tweaks for all
stats kinds that require them.

Andres, do you have any thoughts about that?  Not sure what would be 
your view on this matter.  MemoryContextAllowInCriticalSection() for
stats kinds that allow the case would be OK for me I guess, we should
not be talking about a lot of memory.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (39+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: per backend I/O statistics
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox