public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Andres Freund <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: per backend I/O statistics
Date: Fri, 17 Jan 2025 09:08:02 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CAN55FZ0ZQTf-D+5Rup4H6zp+n3E2EuonYuzTiPkp=1GhesZU_A@mail.gmail.com>
<[email protected]>
<[email protected]>
<66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5>
On Thu, Jan 16, 2025 at 11:28:43AM -0500, Andres Freund wrote:
> On 2025-01-15 18:27:22 +0900, Michael Paquier wrote:
>> My problem is that this is not only related to backend stats, but to
>> all variable-numbered stats kinds that require this behavior.
>
> The issue here imo is that recently IO stats were turned into a variable
> numbered stat, where it previously wasn't. It seems like a bad idea that now
> we regularly do a somewhat sizable memory allocation for backend stats. For
> table stats allocating the pending memory is somewhat a necessity - there
> could be a lot of different tables after all. And later transactions won't
> necessarily access the same set of tables, so we can't just keep the memory
> for pending around - but that's not true for IO.
Yep, for the reason that backend stats require that with one entry in
the pgstats dshash for each backend as these are sized depending on
max_connections & co using a proc number for the hash key.
>> 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.
>
> My view is that for IO stats no memory allocation should be required - that
> used to be the case and should be the case again. And for something like
> injection points (not really convinced that it's a good thing to have stats
> for, but ...), allocation the shared memory stats object outside of the
> critical section and not having any pending stats seems to be the right
> approach.
Not sure to completely agree on that as this pushes the responsibility
of how to deal with the critical section handling to each stats kind,
but I won't fight over it for this case, either. It would not be the
first time we have specific logic to do allocations beforehand when
dealing with critical sections (38c579b08988 is the most recent
example coming in mind, there are many others).
Anyway, let's just switch pgstat_backend.c so as we use a static
PgStat_BackendPending (pending name) to store the pending IO stats
that could be reused for also the WAL bits. It does not seem that
complicated as far as I can see, removing pgstat_prep_backend_pending
and changing pgstat_flush_backend_entry_io to use the static pending
data. The patch set posted here is doing that within its
pgstat_flush_backend_entry_wal():
https://www.postgresql.org/message-id/[email protected]
I could tweak that around the beginning of next week with a proposal
of patch. Bertrand, perhaps you'd prefer hack on this one?
--
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