On Tue, Mar 10, 2020 at 12:15 PM David Steele <da...@pgmasters.net> wrote: > > Hi Jeff, > > On 2/7/20 10:22 AM, Alexey Bashtanov wrote: > > I've changed it all to "unfetched" for at least not to call the same > > thing differently > > in the code and in the output, and also rebased it and fit in 80 lines > > width limit. > > What do you think of Alexey's updates? > > Regards, > -- > -David > da...@pgmasters.net
I've added myself as a reviewer. The patch looks good to me. It doesn't seem to have much risk either; there are not spec concerns applicable (since it's EXPLAIN), and the surface area for impact quite small. Both make check and check-world pass. Here's a test query setup I worked up: create table exp(a int, d int); insert into exp(a, d) select random() * 100, t.i % 50 from generate_series(0,10000000) t(i); create index index_exp_a on exp(a); create index index_exp_d on exp(d); analyze exp; Then: explain analyze select count(*) from exp where a = 25 and d between 5 and 10; shows: Heap Blocks: exact=10518 but if I: vacuum freeze exp; then it shows: Heap Blocks: unfetched=10518 as I'd expect. One question though: if I change the query to: explain (analyze, buffers) select count(*) from exp where a between 50 and 100 and d between 5 and 10; then I get a parallel bitmap heap scan, and I only see exact heap blocks (see attached explain output). Does the original optimization cover parallel bitmap heap scans like this? If not, I think this patch is likely ready for committer. If so, then we still need support for stats tracking and explain output for parallel nodes. I've taken the liberty of: - Reformatting slightly for a cleaner diff. - Running pgindent against the changes - Added a basic commit message. - Add unfetched_pages initialization to ExecInitBitmapHeapScan. See attached. Thanks, James
From 95babe8447eadb40c2d1452a9102d4766269d80f Mon Sep 17 00:00:00 2001
From: James Coleman <jtc...@gmail.com>
Date: Sun, 15 Mar 2020 20:27:19 -0400
Subject: [PATCH v3] Show bitmap only unfetched page count to EXPLAIN
7c70996ebf0949b142a99 added an optimization to bitmap heap scans to
avoid fetching the heap page where the visibility map makes that
possible, much like index only scans.
However that commit didn't add this output to EXPLAIN, so it's not
obvious when the optimization is kicking in and when it's not. So, track
the number of skipped pages and report it in EXPLAIN output.
Author: Jeff Janes
Reviewers: Emre Hasegeli, Alexey Bashtanov, James Coleman
---
src/backend/commands/explain.c | 8 +++++++-
src/backend/executor/nodeBitmapHeapscan.c | 5 +++--
src/include/nodes/execnodes.h | 2 ++
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index d901dc4a50..5a5e04ecbb 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2777,6 +2777,8 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
{
if (es->format != EXPLAIN_FORMAT_TEXT)
{
+ ExplainPropertyInteger("Unfetched Heap Blocks", NULL,
+ planstate->unfetched_pages, es);
ExplainPropertyInteger("Exact Heap Blocks", NULL,
planstate->exact_pages, es);
ExplainPropertyInteger("Lossy Heap Blocks", NULL,
@@ -2784,10 +2786,14 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
}
else
{
- if (planstate->exact_pages > 0 || planstate->lossy_pages > 0)
+ if (planstate->exact_pages > 0 || planstate->lossy_pages > 0
+ || planstate->unfetched_pages > 0)
{
ExplainIndentText(es);
appendStringInfoString(es->str, "Heap Blocks:");
+ if (planstate->unfetched_pages > 0)
+ appendStringInfo(es->str, " unfetched=%ld",
+ planstate->unfetched_pages);
if (planstate->exact_pages > 0)
appendStringInfo(es->str, " exact=%ld", planstate->exact_pages);
if (planstate->lossy_pages > 0)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index ae8a11da30..dce955636f 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -231,14 +231,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
* node->return_empty_tuples.
*/
node->return_empty_tuples = tbmres->ntuples;
+ node->unfetched_pages++;
}
else if (!table_scan_bitmap_next_block(scan, tbmres))
{
/* AM doesn't think this block is valid, skip */
continue;
}
-
- if (tbmres->ntuples >= 0)
+ else if (tbmres->ntuples >= 0)
node->exact_pages++;
else
node->lossy_pages++;
@@ -734,6 +734,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
+ scanstate->unfetched_pages = 0;
scanstate->prefetch_iterator = NULL;
scanstate->prefetch_pages = 0;
scanstate->prefetch_target = 0;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cd3ddf781f..32400be08c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1567,6 +1567,7 @@ typedef struct ParallelBitmapHeapState
* pvmbuffer ditto, for prefetched pages
* exact_pages total number of exact pages retrieved
* lossy_pages total number of lossy pages retrieved
+ * unfetched_pages total number of pages not retrieved due to vm
* prefetch_iterator iterator for prefetching ahead of current page
* prefetch_pages # pages prefetch iterator is ahead of current
* prefetch_target current target prefetch distance
@@ -1591,6 +1592,7 @@ typedef struct BitmapHeapScanState
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
+ long unfetched_pages;
TBMIterator *prefetch_iterator;
int prefetch_pages;
int prefetch_target;
--
2.17.1
explain_parallel_bmp_scan.out
Description: Binary data
