Fix BitmapHeapScan non-parallel-aware EXPLAIN ANALYZE

Allocates shared bitmap table scan instrumentation for all parallel
scans. Previously, the instrumentation was only allocated for
parallel-aware scans, other bitmap heap scans in the parallel query had
no shared instrumentation and EXPLAIN didn't report exact/lossy pages.
This affected cases like scans on the outside of a parallel join or
queries run with debug_parallel_query=regress.

Fixed by allocating a separate DSM chunk for shared instrumentation and
doing so regardless of parallel-awareness. The instrumentation is
allocated in its own DSM chunk, separate from ParallelBitmapHeapState.

Report an initial patch by me. The approach with a separate DSM was
proposed and implemented by Melanie.

Not backpatched. The issue affects Postgres 18 (since 5a1e6df3b84c), but
having multiple DSM chunks is possible only since dd78e69cfc33. If we
decide to fix this in backbranches too, it will need to be done in a
less invasive way.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Reviewed-by: Lukas Fittl <lukas@fittl.com>
Discussion: https://postgr.es/m/flat/a177a6dd-240b-455a-8f25-aca0b1c08c6e%40vondra.me

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 73eaaf176acc4c5418ee06db5e83bb08990c852c..f151f21f9b39c908da0e52cdd8d2027adf7c574f 100644 (file)
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3948,7 +3948,7 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
        }
 
        /* Display stats for each parallel worker */
-       if (planstate->pstate != NULL)
+       if (planstate->sinstrument != NULL)
        {
                for (int n = 0; n < planstate->sinstrument->num_workers; n++)
                {

diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 726aca230a4946976e5d89ec2baef0ab80f35c78..1a5ec0c305f04af6b9289c89d8c403c186938f04 100644 (file)
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -303,6 +303,9 @@ ExecParallelEstimate(PlanState *planstate, ExecParallelEstimateContext *e)
                        if (planstate->plan->parallel_aware)
                                ExecBitmapHeapEstimate((BitmapHeapScanState *) planstate,
                                                                           e->pcxt);
+                       /* even when not parallel-aware, for EXPLAIN ANALYZE */
+                       ExecBitmapHeapInstrumentEstimate((BitmapHeapScanState *) planstate,
+                                                                                        e->pcxt);
                        break;
                case T_HashJoinState:
                        if (planstate->plan->parallel_aware)
@@ -542,6 +545,9 @@ ExecParallelInitializeDSM(PlanState *planstate,
                        if (planstate->plan->parallel_aware)
                                ExecBitmapHeapInitializeDSM((BitmapHeapScanState *) planstate,
                                                                                        d->pcxt);
+                       /* even when not parallel-aware, for EXPLAIN ANALYZE */
+                       ExecBitmapHeapInstrumentInitDSM((BitmapHeapScanState *) planstate,
+                                                                                       d->pcxt);
                        break;
                case T_HashJoinState:
                        if (planstate->plan->parallel_aware)
@@ -1427,6 +1433,9 @@ ExecParallelInitializeWorker(PlanState *planstate, ParallelWorkerContext *pwcxt)
                        if (planstate->plan->parallel_aware)
                                ExecBitmapHeapInitializeWorker((BitmapHeapScanState *) planstate,
                                                                                           pwcxt);
+                       /* even when not parallel-aware, for EXPLAIN ANALYZE */
+                       ExecBitmapHeapInstrumentInitWorker((BitmapHeapScanState *) planstate,
+                                                                                          pwcxt);
                        break;
                case T_HashJoinState:
                        if (planstate->plan->parallel_aware)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 73831aed45119f0ba85d6f485973f4afb1d4dbd1..d65e2a87b42a964a50977911ad1f027f065a14e5 100644 (file)
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -495,18 +495,8 @@ void
 ExecBitmapHeapEstimate(BitmapHeapScanState *node,
                                           ParallelContext *pcxt)
 {
-       Size            size;
-
-       size = MAXALIGN(sizeof(ParallelBitmapHeapState));
-
-       /* account for instrumentation, if required */
-       if (node->ss.ps.instrument && pcxt->nworkers > 0)
-       {
-               size = add_size(size, offsetof(SharedBitmapHeapInstrumentation, sinstrument));
-               size = add_size(size, mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation)));
-       }
-
-       shm_toc_estimate_chunk(&pcxt->estimator, size);
+       shm_toc_estimate_chunk(&pcxt->estimator,
+                                                  MAXALIGN(sizeof(ParallelBitmapHeapState)));
        shm_toc_estimate_keys(&pcxt->estimator, 1);
 }
 
@@ -521,27 +511,15 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
                                                        ParallelContext *pcxt)
 {
        ParallelBitmapHeapState *pstate;
-       SharedBitmapHeapInstrumentation *sinstrument = NULL;
        dsa_area   *dsa = node->ss.ps.state->es_query_dsa;
-       char       *ptr;
-       Size            size;
 
        /* If there's no DSA, there are no workers; initialize nothing. */
        if (dsa == NULL)
                return;
 
-       size = MAXALIGN(sizeof(ParallelBitmapHeapState));
-       if (node->ss.ps.instrument && pcxt->nworkers > 0)
-       {
-               size = add_size(size, offsetof(SharedBitmapHeapInstrumentation, sinstrument));
-               size = add_size(size, mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation)));
-       }
-
-       ptr = shm_toc_allocate(pcxt->toc, size);
-       pstate = (ParallelBitmapHeapState *) ptr;
-       ptr += MAXALIGN(sizeof(ParallelBitmapHeapState));
-       if (node->ss.ps.instrument && pcxt->nworkers > 0)
-               sinstrument = (SharedBitmapHeapInstrumentation *) ptr;
+       pstate = (ParallelBitmapHeapState *)
+               shm_toc_allocate(pcxt->toc,
+                                                MAXALIGN(sizeof(ParallelBitmapHeapState)));
 
        pstate->tbmiterator = 0;
 
@@ -551,18 +529,8 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
 
        ConditionVariableInit(&pstate->cv);
 
-       if (sinstrument)
-       {
-               sinstrument->num_workers = pcxt->nworkers;
-
-               /* ensure any unfilled slots will contain zeroes */
-               memset(sinstrument->sinstrument, 0,
-                          pcxt->nworkers * sizeof(BitmapHeapScanInstrumentation));
-       }
-
        shm_toc_insert(pcxt->toc, node->ss.ps.plan->plan_node_id, pstate);
        node->pstate = pstate;
-       node->sinstrument = sinstrument;
 }
 
 /* ----------------------------------------------------------------
@@ -600,17 +568,72 @@ void
 ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
                                                           ParallelWorkerContext *pwcxt)
 {
-       char       *ptr;
-
        Assert(node->ss.ps.state->es_query_dsa != NULL);
 
-       ptr = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
+       node->pstate = (ParallelBitmapHeapState *)
+               shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
+}
+
+/*
+ * Compute the amount of space we'll need for the shared instrumentation and
+ * inform pcxt->estimator.
+ */
+void
+ExecBitmapHeapInstrumentEstimate(BitmapHeapScanState *node,
+                                                                ParallelContext *pcxt)
+{
+       Size            size;
+
+       if (!node->ss.ps.instrument || pcxt->nworkers == 0)
+               return;
+
+       size = add_size(offsetof(SharedBitmapHeapInstrumentation, sinstrument),
+                                       mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation)));
+       shm_toc_estimate_chunk(&pcxt->estimator, size);
+       shm_toc_estimate_keys(&pcxt->estimator, 1);
+}
+
+/*
+ * Set up parallel bitmap heap scan instrumentation.
+ */
+void
+ExecBitmapHeapInstrumentInitDSM(BitmapHeapScanState *node,
+                                                               ParallelContext *pcxt)
+{
+       Size            size;
+
+       if (!node->ss.ps.instrument || pcxt->nworkers == 0)
+               return;
+
+       size = add_size(offsetof(SharedBitmapHeapInstrumentation, sinstrument),
+                                       mul_size(pcxt->nworkers, sizeof(BitmapHeapScanInstrumentation)));
+       node->sinstrument =
+               (SharedBitmapHeapInstrumentation *) shm_toc_allocate(pcxt->toc, size);
+
+       /* Each per-worker area must start out as zeroes */
+       memset(node->sinstrument, 0, size);
+       node->sinstrument->num_workers = pcxt->nworkers;
+       shm_toc_insert(pcxt->toc,
+                                  node->ss.ps.plan->plan_node_id +
+                                  PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET,
+                                  node->sinstrument);
+}
 
-       node->pstate = (ParallelBitmapHeapState *) ptr;
-       ptr += MAXALIGN(sizeof(ParallelBitmapHeapState));
+/*
+ * Look up and save the location of the shared instrumentation.
+ */
+void
+ExecBitmapHeapInstrumentInitWorker(BitmapHeapScanState *node,
+                                                                  ParallelWorkerContext *pwcxt)
+{
+       if (!node->ss.ps.instrument)
+               return;
 
-       if (node->ss.ps.instrument)
-               node->sinstrument = (SharedBitmapHeapInstrumentation *) ptr;
+       node->sinstrument = (SharedBitmapHeapInstrumentation *)
+               shm_toc_lookup(pwcxt->toc,
+                                          node->ss.ps.plan->plan_node_id +
+                                          PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET,
+                                          false);
 }
 
 /* ----------------------------------------------------------------

diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h
index 5d82f71abff7c42923e09c1f7dd10c53d019bf38..5c0b6592a1f77f9d280aff742ed9a90008d5eb60 100644 (file)
--- a/src/include/executor/nodeBitmapHeapscan.h
+++ b/src/include/executor/nodeBitmapHeapscan.h
@@ -28,6 +28,12 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
                                                                                  ParallelContext *pcxt);
 extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
                                                                                   ParallelWorkerContext *pwcxt);
+extern void ExecBitmapHeapInstrumentEstimate(BitmapHeapScanState *node,
+                                                                                        ParallelContext *pcxt);
+extern void ExecBitmapHeapInstrumentInitDSM(BitmapHeapScanState *node,
+                                                                                       ParallelContext *pcxt);
+extern void ExecBitmapHeapInstrumentInitWorker(BitmapHeapScanState *node,
+                                                                                          ParallelWorkerContext *pwcxt);
 extern void ExecBitmapHeapRetrieveInstrumentation(BitmapHeapScanState *node);
 
 #endif                                                 /* NODEBITMAPHEAPSCAN_H */