From 6b6776c895d3ddb4a9aa0423c41fb71d26f2661e Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 5 Jul 2024 01:33:47 +1200 Subject: [PATCH v7 2/2] fixup! Parallel Bitmap Heap Scan reports per-worker stats --- src/backend/commands/explain.c | 8 ++++-- src/backend/executor/nodeBitmapHeapscan.c | 30 +++++++++++++------- src/include/nodes/execnodes.h | 34 +++++++++++------------ src/tools/pgindent/typedefs.list | 2 ++ 4 files changed, 44 insertions(+), 30 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index d6f66dc5e1..f5634d3c0d 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3628,8 +3628,12 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es) if (es->format == EXPLAIN_FORMAT_TEXT) { ExplainIndentText(es); - appendStringInfo(es->str, "Heap Blocks: exact=%ld lossy=%ld\n", - si->exact_pages, si->lossy_pages); + appendStringInfoString(es->str, "Heap Blocks:"); + if (si->exact_pages > 0) + appendStringInfo(es->str, " exact=%ld", si->exact_pages); + if (si->lossy_pages > 0) + appendStringInfo(es->str, " lossy=%ld", si->lossy_pages); + appendStringInfoChar(es->str, '\n'); } else { diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 973bf56022..3c63bdd93d 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -634,8 +634,18 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) */ if (node->sinstrument != NULL && IsParallelWorker()) { + BitmapHeapScanInstrumentation *si; + Assert(ParallelWorkerNumber <= node->sinstrument->num_workers); - BitmapHeapScanInstrumentation *si = &node->sinstrument->sinstrument[ParallelWorkerNumber]; + si = &node->sinstrument->sinstrument[ParallelWorkerNumber]; + + /* + * Here we accumulate the stats rather than performing memcpy on + * node->stats into si. When a Gather/GatherMerge node finishes it + * will perform planner shutdown on the workers. On rescan it will + * spin up new workers which will have a new BitmapHeapScanState and + * zeroed stats. + */ si->exact_pages += node->stats.exact_pages; si->lossy_pages += node->stats.lossy_pages; } @@ -707,8 +717,10 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->tbmiterator = NULL; scanstate->tbmres = NULL; scanstate->pvmbuffer = InvalidBuffer; - scanstate->stats.exact_pages = 0; - scanstate->stats.lossy_pages = 0; + + /* Zero the statistics counters */ + memset(&scanstate->stats, 0, sizeof(BitmapHeapScanInstrumentation)); + scanstate->prefetch_iterator = NULL; scanstate->prefetch_pages = 0; scanstate->prefetch_target = 0; @@ -818,13 +830,9 @@ ExecBitmapHeapEstimate(BitmapHeapScanState *node, { Size size; - /* - * We store ParallelBitmapHeapState followed by - * SharedBitmapHeapInstrumentationInfo in the same shmem chunk. - */ size = MAXALIGN(sizeof(ParallelBitmapHeapState)); - /* don't need this if not instrumenting or no workers */ + /* account for instrumentation, if required */ if (node->ss.ps.instrument && pcxt->nworkers > 0) { size = add_size(size, offsetof(SharedBitmapHeapInstrumentation, sinstrument)); @@ -879,11 +887,13 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node, ConditionVariableInit(&pstate->cv); - /* ensure any unfilled slots will contain zeroes */ if (sinstrument) { sinstrument->num_workers = pcxt->nworkers; - memset(sinstrument->sinstrument, 0, pcxt->nworkers * sizeof(BitmapHeapScanInstrumentation)); + + /* 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); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index f8a6956ee8..8b1239ff4f 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1746,10 +1746,17 @@ typedef struct BitmapIndexScanState struct IndexScanDescData *biss_ScanDesc; } BitmapIndexScanState; +/* ---------------- + * BitmapHeapScanInstrumentation information + * + * exact_pages total number of exact pages retrieved + * lossy_pages total number of lossy pages retrieved + * ---------------- + */ typedef struct BitmapHeapScanInstrumentation { - long lossy_pages; - long exact_pages; + long exact_pages; + long lossy_pages; } BitmapHeapScanInstrumentation; /* ---------------- @@ -1798,10 +1805,10 @@ typedef struct ParallelBitmapHeapState /* ---------------- * Instrumentation data for a parallel bitmap heap scan. * - * During a parallel bitmap heap scan, this lives in shared memory. At the - * end, each worker copies their own stats to the right slot. Finally, the - * leader copies the data to its local memory - * ---------------- + * A shared memory struct that each parallel worker copies its + * BitmapHeapScanInstrumentation information into at executor shutdown to + * allow the leader to display the information in EXPLAIN ANALYZE. + * ---------------- */ typedef struct SharedBitmapHeapInstrumentation { @@ -1817,8 +1824,7 @@ typedef struct SharedBitmapHeapInstrumentation * tbmiterator iterator for scanning current pages * tbmres current-page data * pvmbuffer buffer for visibility-map lookups of prefetched pages - * exact_pages total number of exact pages retrieved - * lossy_pages total number of lossy pages retrieved + * stats execution statistics * prefetch_iterator iterator for prefetching ahead of current page * prefetch_pages # pages prefetch iterator is ahead of current * prefetch_target current target prefetch distance @@ -1827,6 +1833,7 @@ typedef struct SharedBitmapHeapInstrumentation * shared_tbmiterator shared iterator * shared_prefetch_iterator shared iterator for prefetching * pstate shared state for parallel bitmap scan + * sinstrument statistics for parallel workers * ---------------- */ typedef struct BitmapHeapScanState @@ -1837,6 +1844,7 @@ typedef struct BitmapHeapScanState TBMIterator *tbmiterator; TBMIterateResult *tbmres; Buffer pvmbuffer; + BitmapHeapScanInstrumentation stats; TBMIterator *prefetch_iterator; int prefetch_pages; int prefetch_target; @@ -1845,17 +1853,7 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; - - /* - * In a parallelized bitmap heap scan, each worker sets their - * instrumentaton data in pstate->sinstrument at the end. The leader - * copies the shared-memory info back to local storage before DSM - * shutdown, and stores it here in 'instrument'. It remains NULL in - * workers and in non-parallel scans. - */ SharedBitmapHeapInstrumentation *sinstrument; - - BitmapHeapScanInstrumentation stats; } BitmapHeapScanState; /* ---------------- diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e710fa48e5..3ad15485e9 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -262,6 +262,7 @@ BitmapAndPath BitmapAndState BitmapHeapPath BitmapHeapScan +BitmapHeapScanInstrumentation BitmapHeapScanState BitmapIndexScan BitmapIndexScanState @@ -2603,6 +2604,7 @@ SetToDefault SetupWorkerPtrType ShDependObjectInfo SharedAggInfo +SharedBitmapHeapInstrumentation SharedBitmapState SharedDependencyObjectType SharedDependencyType -- 2.34.1