From: | "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry |
Date: | 2023-10-26 08:04:25 |
Message-ID: | f4db1f6c-73c3-4b88-9d25-dbaa43d3a3a7@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 3/27/23 8:35 AM, Michael Paquier wrote:
> On Fri, Mar 24, 2023 at 08:00:44PM -0700, Andres Freund wrote:
>> I don't understand what we're optimizing for here. These functions are very
>> very very far from being a hot path. The xact functions are barely ever
>> used. Compared to the cost of query evaluation the cost of iterating throught
>> he subxacts is neglegible.
>
> I was wondering about that, and I see why I'm wrong. I have quickly
> gone up to 10k subtransactions, and while I was seeing what looks like
> difference of 8~10% in runtime when looking at
> pg_stat_xact_all_tables, the overval runtime was still close enough
> (5.8ms vs 6.4ms). At this scale, possible that it was some noise,
> these seemed repeatable still not to worry about.
>
> Anyway, I was looking at this patch, and I still feel that it is a bit
> incorrect to have the copy of PgStat_TableStatus returned by
> find_tabstat_entry() to point to the same list of subtransaction data
> as the pending entry found, while the counters are incremented. This
> could lead to mistakes if the copy from find_tabstat_entry() is used
> in an unexpected way in the future. The current callers are OK, but
> this does not give me a warm feeling :/
FWIW, please find attached V7 (mandatory rebase).
It would allow to also define:
- pg_stat_get_xact_tuples_inserted
- pg_stat_get_xact_tuples_updated
- pg_stat_get_xact_tuples_deleted
as macros, joining others pg_stat_get_xact_*() that are already
defined as macros.
The concern you raised above has not been addressed, meaning that
find_tabstat_entry() still return a copy of PgStat_TableStatus.
By "used in an unexpected way in the future", what do you mean exactly? Do you mean
the caller forgetting it is working on a copy and then could work with
"stale" counters?
Trying to understand to see if I should invest time to try to address your concern
or leave those 3 functions as they are currently before moving back to the
"Split index and table statistics into different types of stats" work [1].
[1]: https://www.postgresql.org/message-id/flat/f572abe7-a1bb-e13b-48c7-2ca150546822(at)gmail(dot)com
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v7-0001-Reconcile-stats-in-find_tabstat_entry.patch | text/plain | 4.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Banck | 2023-10-26 08:27:51 | Re: [patch] pg_basebackup: mention that spread checkpoints are the default in --help |
Previous Message | Jelte Fennema | 2023-10-26 08:00:53 | Re: libpq async connection and multiple hosts |