public inbox for [email protected]
help / color / mirror / Atom feedFrom: Drouvot, Bertrand <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
Date: Thu, 16 Mar 2023 14:13:45 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <ZBMBh5ve3tyqH5/[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<ZBMBh5ve3tyqH5/[email protected]>
On 3/16/23 12:46 PM, Michael Paquier wrote:
> On Thu, Mar 16, 2023 at 11:32:56AM +0100, Drouvot, Bertrand wrote:
>> On 3/16/23 7:29 AM, Michael Paquier wrote:
>>> From what I get with this change, the number of tuples changed by DMLs
>>> have their computations done a bit earlier,
>>
>> Thanks for looking at it!
>>
>> Right, but note this is in a dedicated new tablestatus (created
>> within find_tabstat_entry()).
>
> Sure, however it copies the pointer of the PgStat_TableXactStatus from
> tabentry, isn't it?
Oh I see what you mean, yeah, the pointer is copied.
> This means that it keeps a reference of the chain
> of subtransactions. It does not matter for the functions but it could
> for out-of-core callers of find_tabstat_entry(), no?
Yeah, maybe.
> Perhaps you are
> right and that's not worth worrying, still I don't feel particularly
> confident that this is the best approach we can take.
>
due to what potential out-of-core callers could do with it?
>>> How much do we need to care about the remaining two callers
>>> pg_stat_get_xact_blocks_fetched() and pg_stat_get_xact_blocks_hit()?
>>
>> Regarding pg_stat_get_xact_blocks_fetched() and pg_stat_get_xact_blocks_hit()
>> the callers (if any) are outside of the core PG (as from what I can
>> see they are not used at all).
>>
>> I don't think we should pay any particular attention to those 2 ones
>> as anyway nothing prevent the 7 others to be called outside of the
>> pg_stat_xact_all_tables view.
>
> I am not quite sure, TBH. Did you look at the difference with a long
> chain of subtrans, like savepoints? The ODBC driver "loves" producing
> a lot of savepoints, for example.
>
No, I did not measure the impact.
>>> It would feel a bit safer to me to document that find_tabstat_entry()
>>> is currently only used for this xact system view.. The extra
>>> computation could lead to surprises, actually, if this routine is used
>>> outside this context? Perhaps that's OK, but it does not give me a
>>> warm feeling, just to reshape three functions of pgstatfuncs.c with
>>> macros.
>>
>> That's a fair point. On the other hand those 9 functions (which can
>> all be used outside of the pg_stat_xact_all_tables view) are not
>> documented, so I'm not sure this is that much of a concern (and if
>> we think it is we still gave the option to add an extra flag to
>> indicate whether or not the extra computation is needed.)
>
> That's not quite exact, I think. The first 7 functions are used in a
> system catalog that is documented.
Right.
> Still we have a problem here. I
> can actually see a few projects relying on these two functions while
> looking a bit around, so they are used. And the issue comes from
> ddfc2d9, that has removed these functions from the documentation
> ignoring that they are used in no system catalogs. I think that we
> should fix that and re-add the two missing functions with a proper
> description in the docs, at least?
As they could be/are used outside of the xact view, yes I think the same.
> There is no trace of them.
> Perhaps the ones exposted through pg_stat_xact_all_tables are fine if
> not listed.
I'd be tempted to add documentation for all of them, I can look at it.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
view thread (20+ 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]
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
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