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 <[email protected]> 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] > https://www.postgresql.org/message-id/3bdbc70d-ad44-494a-8aab-868b5066fe8b%40vondra.me > > > regards > > -- > Tomas Vondra >
From d57d42e876ab87085593738066f70cbad546aa73 Mon Sep 17 00:00:00 2001 From: Siddharth Kothari <[email protected]> Date: Tue, 17 Feb 2026 12:23:56 +0000 Subject: [PATCH v3] Add parallel instrumentation hooks for CustomScan providers CustomScan providers currently lack a standard method to aggregate instrumentation data from parallel workers back to the leader process before the Dynamic Shared Memory segment is destroyed. Furthermore, unlike built-in scans, they are missing the callbacks required to allocate instrumentation DSM when they are in the parallel part of a plan but not necessarily parallel-aware. This patch introduces four optional callbacks to CustomExecMethods: - EstimateDSMCustomScanInstrumentation - InitializeDSMCustomScanInstrumentation - InitializeWorkerCustomScanInstrumentation - RetrieveInstrumentationCustomScan This allows custom scan providers to fully support parallel EXPLAIN ANALYZE metrics and consolidate instrumentation from shared memory similarly to built-in scan nodes. --- doc/src/sgml/custom-scan.sgml | 43 +++++++++++++++++++++++++++++ src/backend/executor/execParallel.c | 12 ++++++++ src/backend/executor/nodeCustom.c | 37 +++++++++++++++++++++++++ src/include/executor/nodeCustom.h | 7 +++++ src/include/nodes/extensible.h | 13 +++++++++ 5 files changed, 112 insertions(+) diff --git a/doc/src/sgml/custom-scan.sgml b/doc/src/sgml/custom-scan.sgml index a200d502cdd..8c2a4f82041 100644 --- a/doc/src/sgml/custom-scan.sgml +++ b/doc/src/sgml/custom-scan.sgml @@ -409,6 +409,49 @@ void (*ShutdownCustomScan) (CustomScanState *node); <para> <programlisting> +void (*EstimateDSMCustomScanInstrumentation) (CustomScanState *node, + ParallelContext *pcxt); +</programlisting> + Estimate the amount of dynamic shared memory that will be required + for parallel operation's instrumentation (for <literal>EXPLAIN ANALYZE</literal>). + This callback is optional, and need only be supplied if this custom + scan provider supports parallel execution and needs custom instrumentation. + </para> + + <para> +<programlisting> +void (*InitializeDSMCustomScanInstrumentation) (CustomScanState *node, + ParallelContext *pcxt); +</programlisting> + Initialize the dynamic shared memory that will be required for parallel + operation's instrumentation. + This callback is optional, and need only be supplied if this custom + scan provider supports parallel execution and needs custom instrumentation. + </para> + + <para> +<programlisting> +void (*InitializeWorkerCustomScanInstrumentation) (CustomScanState *node, + ParallelWorkerContext *pwcxt); +</programlisting> + Initialize a parallel worker's local state based on the shared state + set up by the leader during <function>InitializeDSMCustomScanInstrumentation</function>. + This callback is optional, and need only be supplied if this custom + scan provider supports parallel execution and needs custom instrumentation. + </para> + + <para> +<programlisting> +void (*RetrieveInstrumentationCustomScan) (CustomScanState *node); +</programlisting> + Retrieve parallel instrumentation data from dynamic shared memory into local + memory when the custom-scan plan node finishes execution. + This callback is optional, and need only be supplied if this custom + scan provider supports parallel execution and needs custom instrumentation. + </para> + + <para> +<programlisting> void (*ExplainCustomScan) (CustomScanState *node, List *ancestors, ExplainState *es); diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 81b87d82fab..7983ac1024e 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -304,6 +304,9 @@ ExecParallelEstimate(PlanState *planstate, ExecParallelEstimateContext *e) if (planstate->plan->parallel_aware) ExecCustomScanEstimate((CustomScanState *) planstate, e->pcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecCustomScanInstrumentEstimate((CustomScanState *) planstate, + e->pcxt); break; case T_BitmapHeapScanState: if (planstate->plan->parallel_aware) @@ -552,6 +555,9 @@ ExecParallelInitializeDSM(PlanState *planstate, if (planstate->plan->parallel_aware) ExecCustomScanInitializeDSM((CustomScanState *) planstate, d->pcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecCustomScanInstrumentInitDSM((CustomScanState *) planstate, + d->pcxt); break; case T_BitmapHeapScanState: if (planstate->plan->parallel_aware) @@ -1166,6 +1172,9 @@ ExecParallelRetrieveInstrumentation(PlanState *planstate, case T_TidRangeScanState: ExecTidRangeScanRetrieveInstrumentation((TidRangeScanState *) planstate); break; + case T_CustomScanState: + ExecCustomScanRetrieveInstrumentation((CustomScanState *) planstate); + break; default: break; } @@ -1451,6 +1460,9 @@ ExecParallelInitializeWorker(PlanState *planstate, ParallelWorkerContext *pwcxt) if (planstate->plan->parallel_aware) ExecCustomScanInitializeWorker((CustomScanState *) planstate, pwcxt); + /* even when not parallel-aware, for EXPLAIN ANALYZE */ + ExecCustomScanInstrumentInitWorker((CustomScanState *) planstate, + pwcxt); break; case T_BitmapHeapScanState: if (planstate->plan->parallel_aware) diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c index b7cc890cd20..04a043543d1 100644 --- a/src/backend/executor/nodeCustom.c +++ b/src/backend/executor/nodeCustom.c @@ -217,6 +217,43 @@ ExecCustomScanInitializeWorker(CustomScanState *node, } } +void +ExecCustomScanInstrumentEstimate(CustomScanState *node, ParallelContext *pcxt) +{ + const CustomExecMethods *methods = node->methods; + + if (methods->EstimateDSMCustomScanInstrumentation) + methods->EstimateDSMCustomScanInstrumentation(node, pcxt); +} + +void +ExecCustomScanInstrumentInitDSM(CustomScanState *node, ParallelContext *pcxt) +{ + const CustomExecMethods *methods = node->methods; + + if (methods->InitializeDSMCustomScanInstrumentation) + methods->InitializeDSMCustomScanInstrumentation(node, pcxt); +} + +void +ExecCustomScanInstrumentInitWorker(CustomScanState *node, + ParallelWorkerContext *pwcxt) +{ + const CustomExecMethods *methods = node->methods; + + if (methods->InitializeWorkerCustomScanInstrumentation) + methods->InitializeWorkerCustomScanInstrumentation(node, pwcxt); +} + +void +ExecCustomScanRetrieveInstrumentation(CustomScanState *node) +{ + const CustomExecMethods *methods = node->methods; + + if (methods->RetrieveInstrumentationCustomScan) + methods->RetrieveInstrumentationCustomScan(node); +} + void ExecShutdownCustomScan(CustomScanState *node) { diff --git a/src/include/executor/nodeCustom.h b/src/include/executor/nodeCustom.h index fb0acc6e414..f702ef01709 100644 --- a/src/include/executor/nodeCustom.h +++ b/src/include/executor/nodeCustom.h @@ -37,6 +37,13 @@ extern void ExecCustomScanReInitializeDSM(CustomScanState *node, ParallelContext *pcxt); extern void ExecCustomScanInitializeWorker(CustomScanState *node, ParallelWorkerContext *pwcxt); +extern void ExecCustomScanInstrumentEstimate(CustomScanState *node, + ParallelContext *pcxt); +extern void ExecCustomScanInstrumentInitDSM(CustomScanState *node, + ParallelContext *pcxt); +extern void ExecCustomScanInstrumentInitWorker(CustomScanState *node, + ParallelWorkerContext *pwcxt); +extern void ExecCustomScanRetrieveInstrumentation(CustomScanState *node); extern void ExecShutdownCustomScan(CustomScanState *node); #endif /* NODECUSTOM_H */ diff --git a/src/include/nodes/extensible.h b/src/include/nodes/extensible.h index 517db95c4a3..69f5a627039 100644 --- a/src/include/nodes/extensible.h +++ b/src/include/nodes/extensible.h @@ -151,6 +151,19 @@ typedef struct CustomExecMethods void *coordinate); void (*ShutdownCustomScan) (CustomScanState *node); + /* Optional: estimate parallel instrumentation size */ + void (*EstimateDSMCustomScanInstrumentation) (CustomScanState *node, + ParallelContext *pcxt); + /* Optional: initialize parallel instrumentation DSM */ + void (*InitializeDSMCustomScanInstrumentation) (CustomScanState *node, + ParallelContext *pcxt); + /* Optional: initialize parallel instrumentation worker */ + void (*InitializeWorkerCustomScanInstrumentation) (CustomScanState *node, + ParallelWorkerContext *pwcxt); + + /* Optional: retrieve parallel instrumentation */ + void (*RetrieveInstrumentationCustomScan) (CustomScanState *node); + /* Optional: print additional information in EXPLAIN */ void (*ExplainCustomScan) (CustomScanState *node, List *ancestors, -- 2.55.0.795.g602f6c329a-goog
