From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
---|---|
To: | Sami Imseih <samimseih(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Lukas Fittl <lukas(at)fittl(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Subject: | Re: Proposal - Allow extensions to set a Plan Identifier |
Date: | 2025-02-13 07:48:04 |
Message-ID: | Z62jtMf9mNX2MSHj@ip-10-97-1-34.eu-west-3.compute.internal |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On Wed, Feb 12, 2025 at 05:44:20PM -0600, Sami Imseih wrote:
> Meanwhile, existing extensions like pg_stat_monitor [3] compute a planId and
> store the plan text, but they lack a way to expose planId in pg_stat_activity.
> This limits their usefulness, as identifying top or long-running plans from
> pg_stat_activity is critical for monitoring.
Agree.
> To address this, I propose leveraging work from [1] and allow extensions
> to set a planId This could be implemented sooner than an in-core
> computed planId.
I think that makes sense and then the idea would be later on to move to something
like 5fd9dfa5f50, but for the "planId": is my understanding correct?
> Proposal Overview:
>
> 1/ Add a planId field to PgBackendStatus.
> 2/ Add a planId field to PlannedStmt.
> 3/ Create APIs for extensions to set the current planId.
Note sure 3 is needed (MyBEEntry is available publicly and PlannedStmt also
through some hooks). But did not look close enough and would probably depends
of the implementation, so let's see.
> Storing planId in PlannedStmt ensures it is available with
> cached plans.
>
> One consideration is whether reserving a 64-bit integer in PgBackendStatus
> and PlannedStmt is acceptable, given that planId may not always be used.
From what I can see PgBackendStatus is currently 432 bytes and PlannedStmt
is 152 bytes. It looks like that adding an uint64 at the end of those structs
would not add extra padding (that's what "ptype /o struct <struct>" tells me)
so would increase to 440 bytes and 160 bytes respectively. I don't think that's
an issue.
> However, this is already the case for queryId when compute_query_id is
> disabled and no extension sets it
Yup.
> Looking forward to feedback on this approach.
> If there is agreement, I will work on preparing the patches.
That sounds like a plan for me but better to wait for others to share their
thoughts too.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Anton A. Melnikov | 2025-02-13 07:48:15 | Re: Change GUC hashtable to use simplehash? |
Previous Message | Michael Paquier | 2025-02-13 07:40:35 | Re: Fix for a crash caused by triggers in cross-partition updates |