| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(at)gmail(dot)com> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, lukas(at)fittl(dot)com, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Improve pg_stat_statements scalability |
| Date: | 2026-07-01 05:17:14 |
| Message-ID: | akSi2txzLZWQL31Q@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, Jun 26, 2026 at 05:33:28PM -0500, Sami Imseih wrote:
> Hi,
>
> Sorry for the long silence, but I have been spending time on
> this and would like to share my updates.
>
> > > On Wed, Jun 03, 2026 at 05:10:39PM +0900, Kyotaro Horiguchi wrote:
> > > > One alternative would be to introduce a separate callback for anytime
> > > > flushes. However, since transaction-boundary flushes ultimately need
> > > > to flush everything anyway, it seems to me that passing a context flag
> > > > would be sufficient.
> > >
> > > Nah. I am not really in favor of an extra callback. I feel that this
> > > could lead to more mistakes when implementing new stats kinds, so
> > > doing your approach of using a value based on the context of the flush
> > > works for me.
>
> > I will proceed with this approach.
>
> After further thought, I don't think passing a context flag to the
> callback is necessary here.
I think this is needed so that custom stats extensions know that it exists (as
they would get a compilation errors and would need to make use of it).
Indeed, if we don't add a flag then it's easy for a custom stats kind to
crash on assert-enabled build or produces weird behavior on non-assert enabled
build. All it has to do is, for example, call GetCurrentTransactionStopTimestamp()
at flush time, say:
static bool
my_extension_flush_pending_cb(PgStat_EntryRef *entry_ref, bool nowait)
{
....
PgStatShared_myextensiontEntry *shared;
....
shared->last_flush_time = GetCurrentTransactionStopTimestamp();
...
}
Then:
BEGIN;
do stuff that would produce the extension to flush stats;
SELECT pg_stat_force_next_flush();
Would:
TRAP: failed Assert("s->state == TRANS_DEFAULT || s->state == TRANS_COMMIT || s->state == TRANS_ABORT || s->state == TRANS_PREPARE"), File: "../src/backend/access/transam/xact.c", Line: 901, PID: 1111147
postgres: postgres postgres [local] SELECT(ExceptionalCondition+0xa4)[0x646bb1bbf2d4]
postgres: postgres postgres [local] SELECT(GetCurrentTransactionStopTimestamp+0x53)[0x646bb1546683]
By adding a flag, extensions owner would be aware that a change is needed.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kirill Reshke | 2026-07-01 05:20:03 | Re: DROP INVALID INDEXES command |
| Previous Message | Feng Wu | 2026-07-01 05:09:15 | Re: [PATCH v2] Avoid internal error for invalid interval typmods |