public inbox for [email protected]
help / color / mirror / Atom feedFrom: Melanie Plageman <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: David Rowley <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
Date: Sun, 5 Apr 2026 14:27:54 -0400
Message-ID: <CAAKRu_a_c8HAtJ8Ynz-dU=Jb2PzheW0zWME6A1BB9jQ62DMZBg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAFiTN-v1yDvU=X+hwfJ+55=sbgDj=_kuvbduEG-F7=BjpWcnuw@mail.gmail.com>
<[email protected]>
<CAA=D8a1ZGWTk0RDH-LyU==RsE-217T29eprS-NBhyj+eLHyRrQ@mail.gmail.com>
<CAAKRu_YjBPfGp85ehY1t9NN=R9pB9k=6rztaeVkAm-OeTqUK4g@mail.gmail.com>
<CAA=D8a1G5CgvdWLd5_VCEc5Sa3rgJftcGfTEvYqG1HnjMBQmpA@mail.gmail.com>
<[email protected]>
<TYWPR01MB10982C38F6BD6BE4D1D5CCC5EB1D62@TYWPR01MB10982.jpnprd01.prod.outlook.com>
<CAApHDvqsTB-zNw_SpcujnYCi6FYN3M16kJn9QX7pHtVq5wP_Lw@mail.gmail.com>
<CAApHDvpAz_wVNXeCEhjyxcXM6yXMhs+wG+3vywDaJptj6w7dRA@mail.gmail.com>
<CAApHDvpS_97TU+jWPc=T83WPp7vJa1dTw3mojEtAVEZOWh9bjQ@mail.gmail.com>
<CAApHDvo+mqzBNW3YX2EnK0P+Y=f=+2VpWXdHF477susd-bk+-A@mail.gmail.com>
<[email protected]>
On Fri, Apr 3, 2026 at 3:20 PM Tomas Vondra <[email protected]> wrote:
>
> I'm working on adding information about prefetching for scans [1], which
> includes BitmapHeapScan. I realized the instrumentation added by this
> thread may not be quite right, resulting in missing instrumentation for
> non-parallel-aware scans in a parallel query.
>
> A better description / explanation of the issue is posted here [2]. I've
> posted a proposed fix in the following message [3], in a patch:
>
> v8-0002-Show-Bitmap-Heap-Scan-stats-for-non-parallel-awar.patch
>
> I wonder if someone from this thread could review my analysis, and
> confirm this is not intentional. I don't see it discussed in the thread,
> so I assume no one noticed this behavior. I'd also appreciate a review
> of the proposed fix, or suggestions for alternative fixes.
I can't imagine this was intentional.
I reviewed your approach and suggest we aim for even lower impact by
always allocating the ParallelBitmapHeapState. That means the DSM
layout won't differ such that pcxt->toc has to point to the
instrumentation in the parallel-oblivious case and the pstate in the
parallel-aware case. Attached is a patch that does this.
- Melanie
Attachments:
[text/x-patch] 0001-Allow-non-parallel-aware-bitmap-table-scans-to-share.patch (6.3K, 2-0001-Allow-non-parallel-aware-bitmap-table-scans-to-share.patch)
download | inline diff:
From 695215d4ada297ca31034c3c13f4d491f0c25a9a Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Sun, 5 Apr 2026 14:10:33 -0400
Subject: [PATCH] Allow non-parallel-aware bitmap table scans to share
instrumentation in the DSM
EXPLAIN ANALYZE for non-parallel-aware bitmap table scans did not show
exact/lossy pages because the DSM where the stats would be read from
wasn't initialized. This affected queries like bitmap table scans on the
outer side of a parallel join or bitmap table scans with
debug_parallel_query=regress. Fix it by setting up the DSM if
instrumentation is needed even if the node is not parallel aware.
---
src/backend/commands/explain.c | 2 +-
src/backend/executor/execParallel.c | 18 ++++----
src/backend/executor/nodeBitmapHeapscan.c | 50 +++++++++++++++--------
3 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e4b70166b0e..8275bb2af61 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3949,7 +3949,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 755191b51ef..ce377a774f8 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -290,9 +290,9 @@ ExecParallelEstimate(PlanState *planstate, ExecParallelEstimateContext *e)
e->pcxt);
break;
case T_BitmapHeapScanState:
- if (planstate->plan->parallel_aware)
- ExecBitmapHeapEstimate((BitmapHeapScanState *) planstate,
- e->pcxt);
+ /* even when not parallel-aware, for EXPLAIN ANALYZE */
+ ExecBitmapHeapEstimate((BitmapHeapScanState *) planstate,
+ e->pcxt);
break;
case T_HashJoinState:
if (planstate->plan->parallel_aware)
@@ -522,9 +522,9 @@ ExecParallelInitializeDSM(PlanState *planstate,
d->pcxt);
break;
case T_BitmapHeapScanState:
- if (planstate->plan->parallel_aware)
- ExecBitmapHeapInitializeDSM((BitmapHeapScanState *) planstate,
- d->pcxt);
+ /* even when not parallel-aware, for EXPLAIN ANALYZE */
+ ExecBitmapHeapInitializeDSM((BitmapHeapScanState *) planstate,
+ d->pcxt);
break;
case T_HashJoinState:
if (planstate->plan->parallel_aware)
@@ -1400,9 +1400,9 @@ ExecParallelInitializeWorker(PlanState *planstate, ParallelWorkerContext *pwcxt)
pwcxt);
break;
case T_BitmapHeapScanState:
- if (planstate->plan->parallel_aware)
- ExecBitmapHeapInitializeWorker((BitmapHeapScanState *) planstate,
- pwcxt);
+ /* even when not parallel-aware, for EXPLAIN ANALYZE */
+ ExecBitmapHeapInitializeWorker((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 73831aed451..7ae348ef1f1 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -103,9 +103,9 @@ BitmapTableScanSetup(BitmapHeapScanState *node)
{
TBMIterator tbmiterator = {0};
ParallelBitmapHeapState *pstate = node->pstate;
- dsa_area *dsa = node->ss.ps.state->es_query_dsa;
+ bool parallel_aware = node->ss.ps.plan->parallel_aware;
- if (!pstate)
+ if (!parallel_aware)
{
node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -133,8 +133,9 @@ BitmapTableScanSetup(BitmapHeapScanState *node)
BitmapDoneInitializingSharedState(pstate);
}
- tbmiterator = tbm_begin_iterate(node->tbm, dsa,
- pstate ?
+ tbmiterator = tbm_begin_iterate(node->tbm,
+ node->ss.ps.state->es_query_dsa,
+ parallel_aware ?
pstate->tbmiterator :
InvalidDsaPointer);
@@ -497,6 +498,12 @@ ExecBitmapHeapEstimate(BitmapHeapScanState *node,
{
Size size;
+ if (!node->ss.ps.instrument && !node->ss.ps.plan->parallel_aware)
+ {
+ /* No DSM required by the scan */
+ return;
+ }
+
size = MAXALIGN(sizeof(ParallelBitmapHeapState));
/* account for instrumentation, if required */
@@ -522,13 +529,14 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
{
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)
+ if (!node->ss.ps.instrument && !node->ss.ps.plan->parallel_aware)
+ {
+ /* No DSM required by the scan */
return;
+ }
size = MAXALIGN(sizeof(ParallelBitmapHeapState));
if (node->ss.ps.instrument && pcxt->nworkers > 0)
@@ -543,13 +551,18 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
if (node->ss.ps.instrument && pcxt->nworkers > 0)
sinstrument = (SharedBitmapHeapInstrumentation *) ptr;
- pstate->tbmiterator = 0;
-
- /* Initialize the mutex */
- SpinLockInit(&pstate->mutex);
- pstate->state = BM_INITIAL;
+ pstate->tbmiterator = InvalidDsaPointer;
- ConditionVariableInit(&pstate->cv);
+ /*
+ * Only initialize these fields when parallel-aware as they are used to
+ * coordinate TBM iteration amongst parallel workers.
+ */
+ if (node->ss.ps.plan->parallel_aware)
+ {
+ SpinLockInit(&pstate->mutex);
+ pstate->state = BM_INITIAL;
+ ConditionVariableInit(&pstate->cv);
+ }
if (sinstrument)
{
@@ -578,9 +591,8 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
ParallelBitmapHeapState *pstate = node->pstate;
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
- /* If there's no DSA, there are no workers; do nothing. */
- if (dsa == NULL)
- return;
+ Assert(node->ss.ps.plan->parallel_aware);
+ Assert(dsa != NULL);
pstate->state = BM_INITIAL;
@@ -602,7 +614,11 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
{
char *ptr;
- Assert(node->ss.ps.state->es_query_dsa != NULL);
+ if (!node->ss.ps.instrument && !node->ss.ps.plan->parallel_aware)
+ {
+ /* No DSM required by the scan */
+ return;
+ }
ptr = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
--
2.43.0
view thread (23+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
In-Reply-To: <CAAKRu_a_c8HAtJ8Ynz-dU=Jb2PzheW0zWME6A1BB9jQ62DMZBg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox