[PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Started by Siddharth Kothari5 months ago7 messageshackers
Jump to latest
#1Siddharth Kothari
sidkot@google.com

Dear PostgreSQL Hackers,

This email proposes a patch to enhance the CustomScan provider interface.
The patch file,
0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patch, is
attached.

*Problem:*

CustomScan providers currently lack a standard method to aggregate
instrumentation data from parallel workers back to the leader process
before the Dynamic Shared Memory (DSM) segment is unlinked. This makes it
difficult to gather comprehensive performance metrics from parallelized
custom scans.

*Solution:*

This patch introduces a new optional hook, RetrieveInstrumentationCustomScan,
to the CustomExecMethods struct. This hook allows custom scan providers to
implement logic to collect and consolidate instrumentation from shared
memory or worker states during the parallel query cleanup phase. This hook
is invoked via the new ExecCustomScanRetrieveInstrumentation function,
called from ExecParallelRetrieveInstrumentation for T_CustomScanState nodes.
Since the hook is optional (checked for NULL before calling), it maintains
full backward compatibility.

*Testing & Compatibility:*

- The patch compiles and passes all core regression tests (make
check-world) on my x86_64 instance.
- The changes are not platform-specific.
- Regression Tests: This patch provides a new *capability* for custom
scan providers. Since the hook's functionality is only realized when
implemented by an extension, specific tests would naturally reside within
that extension rather than in the core regression suite.

This patch does not directly address a specific item on the official TODO
list but enhances the extensibility framework.

I believe this patch is complete and ready for review. I look forward to
any feedback and am happy to make revisions. I will also add this patch to
the next CommitFest.

Thank you,

Siddharth Kothari

Attachments:

0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patchapplication/x-patch; name=0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patchDownload+16-1
#2Siddharth Kothari
sidkot@google.com
In reply to: Siddharth Kothari (#1)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi everyone,

I’m just checking in to see if anyone has had a chance to look at this or
if there’s any further information I should provide to help with the
review. I have also added the patch to PG20-1 CF queue, the link is
https://commitfest.postgresql.org/patch/6524/.

Thanks,
Siddharth

On Wed, Feb 18, 2026 at 3:09 PM Siddharth Kothari <sidkot@google.com> wrote:

Show quoted text

Dear PostgreSQL Hackers,

This email proposes a patch to enhance the CustomScan provider interface.
The patch file,
0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patch, is
attached.

*Problem:*

CustomScan providers currently lack a standard method to aggregate
instrumentation data from parallel workers back to the leader process
before the Dynamic Shared Memory (DSM) segment is unlinked. This makes it
difficult to gather comprehensive performance metrics from parallelized
custom scans.

*Solution:*

This patch introduces a new optional hook,
RetrieveInstrumentationCustomScan, to the CustomExecMethods struct. This
hook allows custom scan providers to implement logic to collect and
consolidate instrumentation from shared memory or worker states during the
parallel query cleanup phase. This hook is invoked via the new
ExecCustomScanRetrieveInstrumentation function, called from
ExecParallelRetrieveInstrumentation for T_CustomScanState nodes. Since
the hook is optional (checked for NULL before calling), it maintains full
backward compatibility.

*Testing & Compatibility:*

- The patch compiles and passes all core regression tests (make
check-world) on my x86_64 instance.
- The changes are not platform-specific.
- Regression Tests: This patch provides a new *capability* for custom
scan providers. Since the hook's functionality is only realized when
implemented by an extension, specific tests would naturally reside within
that extension rather than in the core regression suite.

This patch does not directly address a specific item on the official TODO
list but enhances the extensibility framework.

I believe this patch is complete and ready for review. I look forward to
any feedback and am happy to make revisions. I will also add this patch to
the next CommitFest.

Thank you,

Siddharth Kothari

#3Siddharth Kothari
sidkot@google.com
In reply to: Siddharth Kothari (#2)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi hackers,

Bumping this thread as this small patch is passing all CF checks but needs
a reviewer.

*The Problem:* Right now, if you write a parallel CustomScan extension, it
is impossible to get performance metrics from parallel workers. The data is
destroyed during DSM unlinking before the leader can grab it.

*The Fix:* This patch adds an optional RetrieveInstrumentation hook to
CustomExecMethods. It has zero overhead for existing extensions (it's a
simple NULL check) but unlocks metric aggregation for parallel custom scans.

Please let me know your thoughts.

Thanks,

Siddharth

On Wed, Apr 8, 2026 at 10:57 AM Siddharth Kothari <sidkot@google.com> wrote:

Show quoted text

Hi everyone,

I’m just checking in to see if anyone has had a chance to look at this or
if there’s any further information I should provide to help with the
review. I have also added the patch to PG20-1 CF queue, the link is
https://commitfest.postgresql.org/patch/6524/.

Thanks,
Siddharth

On Wed, Feb 18, 2026 at 3:09 PM Siddharth Kothari <sidkot@google.com>
wrote:

Dear PostgreSQL Hackers,

This email proposes a patch to enhance the CustomScan provider interface.
The patch file,
0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patch, is
attached.

*Problem:*

CustomScan providers currently lack a standard method to aggregate
instrumentation data from parallel workers back to the leader process
before the Dynamic Shared Memory (DSM) segment is unlinked. This makes it
difficult to gather comprehensive performance metrics from parallelized
custom scans.

*Solution:*

This patch introduces a new optional hook,
RetrieveInstrumentationCustomScan, to the CustomExecMethods struct. This
hook allows custom scan providers to implement logic to collect and
consolidate instrumentation from shared memory or worker states during the
parallel query cleanup phase. This hook is invoked via the new
ExecCustomScanRetrieveInstrumentation function, called from
ExecParallelRetrieveInstrumentation for T_CustomScanState nodes. Since
the hook is optional (checked for NULL before calling), it maintains full
backward compatibility.

*Testing & Compatibility:*

- The patch compiles and passes all core regression tests (make
check-world) on my x86_64 instance.
- The changes are not platform-specific.
- Regression Tests: This patch provides a new *capability* for custom
scan providers. Since the hook's functionality is only realized when
implemented by an extension, specific tests would naturally reside within
that extension rather than in the core regression suite.

This patch does not directly address a specific item on the official TODO
list but enhances the extensibility framework.

I believe this patch is complete and ready for review. I look forward to
any feedback and am happy to make revisions. I will also add this patch to
the next CommitFest.

Thank you,

Siddharth Kothari

Attachments:

0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patchapplication/octet-stream; name=0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patchDownload+16-1
#4Siddharth Kothari
sidkot@google.com
In reply to: Siddharth Kothari (#3)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi hackers,

Adding Melanie and Tomas to this thread. I noticed you both recently
committed similar changes to handle shared memory instrumentation
separation and retrieval for IndexScan/IndexOnlyScan (dd78e69cfc
<https://github.com/postgres/postgres/commit/dd78e69cfc337f93cfc0303ddf660262d7f1237e&gt;)
and SeqScan (3b1117d6e2
<https://github.com/postgres/postgres/commit/3b1117d6e2e47d86cdbd978b79434c630cb0ef52&gt;
).

My patch implements the exact same pattern for CustomScan states. It adds
an optional RetrieveInstrumentationCustomScan hook so that extensions can
aggregate their worker metrics before the DSM gets unlinked, bringing
custom scans to parity with the recent core scan instrumentation
improvements.

Could you please help review?

The CommitFest entry is also updated here:
https://commitfest.postgresql.org/patch/6524/

Thanks, Siddharth

On Tue, Jun 23, 2026 at 2:38 PM Siddharth Kothari <sidkot@google.com> wrote:

Show quoted text

Hi hackers,

Bumping this thread as this small patch is passing all CF checks but needs
a reviewer.

*The Problem:* Right now, if you write a parallel CustomScan extension,
it is impossible to get performance metrics from parallel workers. The data
is destroyed during DSM unlinking before the leader can grab it.

*The Fix:* This patch adds an optional RetrieveInstrumentation hook to
CustomExecMethods. It has zero overhead for existing extensions (it's a
simple NULL check) but unlocks metric aggregation for parallel custom scans.

Please let me know your thoughts.

Thanks,

Siddharth

On Wed, Apr 8, 2026 at 10:57 AM Siddharth Kothari <sidkot@google.com>
wrote:

Hi everyone,

I’m just checking in to see if anyone has had a chance to look at this or
if there’s any further information I should provide to help with the
review. I have also added the patch to PG20-1 CF queue, the link is
https://commitfest.postgresql.org/patch/6524/.

Thanks,
Siddharth

On Wed, Feb 18, 2026 at 3:09 PM Siddharth Kothari <sidkot@google.com>
wrote:

Dear PostgreSQL Hackers,

This email proposes a patch to enhance the CustomScan provider
interface. The patch file,
0001-Add-RetrieveInstrumentationCustomScan-hook-for-Custo.patch, is
attached.

*Problem:*

CustomScan providers currently lack a standard method to aggregate
instrumentation data from parallel workers back to the leader process
before the Dynamic Shared Memory (DSM) segment is unlinked. This makes it
difficult to gather comprehensive performance metrics from parallelized
custom scans.

*Solution:*

This patch introduces a new optional hook,
RetrieveInstrumentationCustomScan, to the CustomExecMethods struct.
This hook allows custom scan providers to implement logic to collect and
consolidate instrumentation from shared memory or worker states during the
parallel query cleanup phase. This hook is invoked via the new
ExecCustomScanRetrieveInstrumentation function, called from
ExecParallelRetrieveInstrumentation for T_CustomScanState nodes. Since
the hook is optional (checked for NULL before calling), it maintains full
backward compatibility.

*Testing & Compatibility:*

- The patch compiles and passes all core regression tests (make
check-world) on my x86_64 instance.
- The changes are not platform-specific.
- Regression Tests: This patch provides a new *capability* for
custom scan providers. Since the hook's functionality is only realized when
implemented by an extension, specific tests would naturally reside within
that extension rather than in the core regression suite.

This patch does not directly address a specific item on the official
TODO list but enhances the extensibility framework.

I believe this patch is complete and ready for review. I look forward to
any feedback and am happy to make revisions. I will also add this patch to
the next CommitFest.

Thank you,

Siddharth Kothari

Attachments:

v2-0001-Add-RetrieveInstrumentationCustomScan-hook-for-Cu.patchapplication/x-patch; name=v2-0001-Add-RetrieveInstrumentationCustomScan-hook-for-Cu.patchDownload+16-1
#5Tomas Vondra
tomas.vondra@2ndquadrant.com
In reply to: Siddharth Kothari (#4)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi Siddharth,

On 7/8/26 18:52, Siddharth Kothari wrote:

Hi hackers,

Adding Melanie and Tomas to this thread. I noticed you both recently
committed similar changes to handle shared memory instrumentation
separation and retrieval for IndexScan/IndexOnlyScan (dd78e69cfc
<https://github.com/postgres/postgres/commit/
dd78e69cfc337f93cfc0303ddf660262d7f1237e>) and SeqScan (3b1117d6e2
<https://github.com/postgres/postgres/
commit/3b1117d6e2e47d86cdbd978b79434c630cb0ef52>).

My patch implements the exact same pattern for CustomScan states. It
adds an optional RetrieveInstrumentationCustomScan hook so that
extensions can aggregate their worker metrics before the DSM gets
unlinked, bringing custom scans to parity with the recent core scan
instrumentation improvements.

Could you please help review?

The CommitFest entry is also updated here: https://
commitfest.postgresql.org/patch/6524/ <https://
commitfest.postgresql.org/patch/6524/>

Thanks for the patch, and sorry for not responding earlier. The last
couple months were incredibly busy, both because of the last commitfest
and personal reasons.

I think the patch looks generally OK, except for two things:

1) It needs to add the new callback to doc/src/sgml/custom-scan.sgml,
with similar documentation as for the other callbacks.

2) While looking at execParallel.c it occurred to me the CustomScan may
have the same issue as described in [1]/messages/by-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b@vondra.me (and a couple messages after
that). The scan may in the parallel part of a plan, but not necessarily
parallel-aware. But a lot of the initialization is gated by

if (planstate->plan->parallel_aware)
...

All the other scans initialize some of the instrumentation always, in
blocks like this:

/* even when not parallel-aware, for EXPLAIN ANALYZE */

And BHS had this issue until 9c18b47e610, but we didn't do the same
thing for CustomScan. I guess we should.

Of course, it's not the fault of this patch, it's just something I
noticed while looking at the code. OTOH it probably also makes sense for
CustomScan to handle the instrumentation just like the other scans, i.e.
by allocating a separate DSM for the instrumentation. AFAICS that'll
require a couple more optional callbacks in CustomExecMethods:

ExecCustomScanInstrumentEstimate
ExecCustomScanInstrumentInitDSM
ExecCustomScanInstrumentInitWorker

But I think that's OK.

[1]: /messages/by-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b@vondra.me
/messages/by-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b@vondra.me

regards

--
Tomas Vondra

#6Siddharth Kothari
sidkot@google.com
In reply to: Tomas Vondra (#5)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi Tomas,

Thanks for the review, and no worries.
I completely agree with both of your suggestions.

I added the new missing callbacks:

ExecCustomScanInstrumentEstimate
ExecCustomScanInstrumentInitDSM
ExecCustomScanInstrumentInitWorker
along with the existing
ExecCustomScanRetrieveInstrumentation

and extracted the custom scan instrumentation tracking out of the
parallel_aware guard block in execParallel.c. The core execution
orchestrator will now correctly invoke these instrumentation callbacks
even when a CustomScan node is part of a parallel chunk but not
strictly parallel_aware itself.

I also updated the Custom scan documentation to properly document the
new callbacks.

I've attached the v3 patch with these changes included. Let me know if
everything looks good or if there is anything else I should adjust.

Regards,
Siddharth

Show quoted text

On Thu, Jul 9, 2026 at 12:10 AM Tomas Vondra <tomas@vondra.me> wrote:

Hi Siddharth,

On 7/8/26 18:52, Siddharth Kothari wrote:

Hi hackers,

Adding Melanie and Tomas to this thread. I noticed you both recently
committed similar changes to handle shared memory instrumentation
separation and retrieval for IndexScan/IndexOnlyScan (dd78e69cfc
<https://github.com/postgres/postgres/commit/
dd78e69cfc337f93cfc0303ddf660262d7f1237e>) and SeqScan (3b1117d6e2
<https://github.com/postgres/postgres/
commit/3b1117d6e2e47d86cdbd978b79434c630cb0ef52>).

My patch implements the exact same pattern for CustomScan states. It
adds an optional RetrieveInstrumentationCustomScan hook so that
extensions can aggregate their worker metrics before the DSM gets
unlinked, bringing custom scans to parity with the recent core scan
instrumentation improvements.

Could you please help review?

The CommitFest entry is also updated here: https://
commitfest.postgresql.org/patch/6524/ <https://
commitfest.postgresql.org/patch/6524/>

Thanks for the patch, and sorry for not responding earlier. The last
couple months were incredibly busy, both because of the last commitfest
and personal reasons.

I think the patch looks generally OK, except for two things:

1) It needs to add the new callback to doc/src/sgml/custom-scan.sgml,
with similar documentation as for the other callbacks.

2) While looking at execParallel.c it occurred to me the CustomScan may
have the same issue as described in [1] (and a couple messages after
that). The scan may in the parallel part of a plan, but not necessarily
parallel-aware. But a lot of the initialization is gated by

if (planstate->plan->parallel_aware)
...

All the other scans initialize some of the instrumentation always, in
blocks like this:

/* even when not parallel-aware, for EXPLAIN ANALYZE */

And BHS had this issue until 9c18b47e610, but we didn't do the same
thing for CustomScan. I guess we should.

Of course, it's not the fault of this patch, it's just something I
noticed while looking at the code. OTOH it probably also makes sense for
CustomScan to handle the instrumentation just like the other scans, i.e.
by allocating a separate DSM for the instrumentation. AFAICS that'll
require a couple more optional callbacks in CustomExecMethods:

ExecCustomScanInstrumentEstimate
ExecCustomScanInstrumentInitDSM
ExecCustomScanInstrumentInitWorker

But I think that's OK.

[1]
/messages/by-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b@vondra.me

regards

--
Tomas Vondra

Attachments:

v3-0001-Add-parallel-instrumentation-hooks-for-CustomScan.patchapplication/x-patch; name=v3-0001-Add-parallel-instrumentation-hooks-for-CustomScan.patchDownload+112-5
#7Siddharth Kothari
sidkot@google.com
In reply to: Siddharth Kothari (#6)
Re: [PATCH] Add RetrieveInstrumentation hook for CustomScan providers

Hi Tomas,

Just sending a gentle ping on this thread. Please let me know if you
get a chance to take a look at the v3 patch, or if there is anything
else you'd like me to address!

Thanks,
Siddharth

Show quoted text

On Thu, Jul 9, 2026 at 1:37 PM Siddharth Kothari <sidkot@google.com> wrote:

Hi Tomas,

Thanks for the review, and no worries.
I completely agree with both of your suggestions.

I added the new missing callbacks:

ExecCustomScanInstrumentEstimate
ExecCustomScanInstrumentInitDSM
ExecCustomScanInstrumentInitWorker
along with the existing
ExecCustomScanRetrieveInstrumentation

and extracted the custom scan instrumentation tracking out of the
parallel_aware guard block in execParallel.c. The core execution
orchestrator will now correctly invoke these instrumentation callbacks
even when a CustomScan node is part of a parallel chunk but not
strictly parallel_aware itself.

I also updated the Custom scan documentation to properly document the
new callbacks.

I've attached the v3 patch with these changes included. Let me know if
everything looks good or if there is anything else I should adjust.

Regards,
Siddharth

On Thu, Jul 9, 2026 at 12:10 AM Tomas Vondra <tomas@vondra.me> wrote:

Hi Siddharth,

On 7/8/26 18:52, Siddharth Kothari wrote:

Hi hackers,

Adding Melanie and Tomas to this thread. I noticed you both recently
committed similar changes to handle shared memory instrumentation
separation and retrieval for IndexScan/IndexOnlyScan (dd78e69cfc
<https://github.com/postgres/postgres/commit/
dd78e69cfc337f93cfc0303ddf660262d7f1237e>) and SeqScan (3b1117d6e2
<https://github.com/postgres/postgres/
commit/3b1117d6e2e47d86cdbd978b79434c630cb0ef52>).

My patch implements the exact same pattern for CustomScan states. It
adds an optional RetrieveInstrumentationCustomScan hook so that
extensions can aggregate their worker metrics before the DSM gets
unlinked, bringing custom scans to parity with the recent core scan
instrumentation improvements.

Could you please help review?

The CommitFest entry is also updated here: https://
commitfest.postgresql.org/patch/6524/ <https://
commitfest.postgresql.org/patch/6524/>

Thanks for the patch, and sorry for not responding earlier. The last
couple months were incredibly busy, both because of the last commitfest
and personal reasons.

I think the patch looks generally OK, except for two things:

1) It needs to add the new callback to doc/src/sgml/custom-scan.sgml,
with similar documentation as for the other callbacks.

2) While looking at execParallel.c it occurred to me the CustomScan may
have the same issue as described in [1] (and a couple messages after
that). The scan may in the parallel part of a plan, but not necessarily
parallel-aware. But a lot of the initialization is gated by

if (planstate->plan->parallel_aware)
...

All the other scans initialize some of the instrumentation always, in
blocks like this:

/* even when not parallel-aware, for EXPLAIN ANALYZE */

And BHS had this issue until 9c18b47e610, but we didn't do the same
thing for CustomScan. I guess we should.

Of course, it's not the fault of this patch, it's just something I
noticed while looking at the code. OTOH it probably also makes sense for
CustomScan to handle the instrumentation just like the other scans, i.e.
by allocating a separate DSM for the instrumentation. AFAICS that'll
require a couple more optional callbacks in CustomExecMethods:

ExecCustomScanInstrumentEstimate
ExecCustomScanInstrumentInitDSM
ExecCustomScanInstrumentInitWorker

But I think that's OK.

[1]
/messages/by-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b@vondra.me

regards

--
Tomas Vondra