public inbox for [email protected]
help / color / mirror / Atom feedFrom: 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]
Subject: Re: per backend I/O statistics
Date: Tue, 19 Nov 2024 10:47:53 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<ZypXy3fH/[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On Thu, Nov 14, 2024 at 01:30:11PM +0000, Bertrand Drouvot wrote:
> - change the arguments to false in the pgstat_drop_entry_internal() call in
> pgstat_drop_all_entries()
> - start the engine
> - kill -9 postgres
> - restart the engine
>
> You'll see the assert failing due to the startup process. I don't think it is
> doing something wrong though, just populating its backend stats. Do you have
> another view on it?
It feels sad that we have to plug in the internals at this level for
this particular case. Perhaps there is something to do with more
callbacks. Or perhaps there is just no point in tracking the stats of
auxiliary processes because we already have this data in the existing
pg_stat_io already?
> How would that work for someone doing "select * from pg_stat_get_backend_io(<pid>)"?
>
> Do you mean copy/paste part of the code from pg_stat_get_activity() into
> pg_stat_get_backend_io() to get the backend type? That sounds like an idea,
> I'll have a look at it.
I was under the impression that we should keep PgStat_Backend for the
reset_timestamp part, but drop BackendType as it can be guessed from
pg_stat_activity itself. For example a LATERAL to grab the current
live stats of these backends.
>> +typedef struct PgStat_PendingIO
>>
>> Perhaps this part should use a separate structure named
>> "BackendPendingIO"? The definition of the structure has to be in
>> pgstat.h as this is the pending_size of the new stats kind. It looks
>> like it would be cleaner to keep PgStat_PendingIO local to
>> pgstat_io.c, and define PgStat_PendingIO based on
>> PgStat_BackendPendingIO?
>
> I see what you meean, what about simply "PgStat_BackendPending" in pgstat.h?
That should be OK.
>> Structurally, it may be cleaner to keep all the callbacks and the
>> backend-I/O specific logic into a separate file, perhaps
>> pgstat_io_backend.c or pgstat_backend_io?
>
> Yeah, I was wondering the same. What about pgstat_backend.c (that would contain
> only one "section" dedicated to IO currently)?
pgstat_backend.c looks good to me. Could there be other stats than
just IO, actually? Perhaps not, just asking..
> - if the idea is to 1. record checkpointer stats, 2. do stuff in the backend and
> 3. check again the checkpointer stats then I don't think setting stats_fetch_consistency
> to snapshot is needed at all. In fact it's quite the opposite as 1. and 3. would
> report the same values with stats_fetch_consistency set to 'snapshot' (if done
> in the same transaction).
Hmm. Not sure. It looks like you're right to treat this in a
separate patch as it would exist for different reasons than the
original proposal.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (31+ 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]
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