public inbox for [email protected]
help / color / mirror / Atom feedFrom: Melanie Plageman <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Date: Fri, 5 Apr 2024 04:06:34 -0400
Message-ID: <20240405080634.d7c23f46xjhhxf5q@liskov> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAAKRu_ZS016J_qhs=BnCsteYYC0HgYpAbZB8mDZBvFdy_VD1MQ@mail.gmail.com>
<20240325160709.qjo6txzu6zjxzkqy@liskov>
<20240327193750.3mlcmzqondpj27xe@liskov>
<[email protected]>
<CA+hUKGJtm_gkmW_h_02-Q9ZRcG3yOx2uzVqbCTfz7YPnTfs+DA@mail.gmail.com>
<[email protected]>
<20240331154551.lty4mundyoyhtixw@liskov>
<20240403225759.sq5hmxdmbalroosy@liskov>
<[email protected]>
On Thu, Apr 04, 2024 at 04:35:45PM +0200, Tomas Vondra wrote:
>
>
> On 4/4/24 00:57, Melanie Plageman wrote:
> > On Sun, Mar 31, 2024 at 11:45:51AM -0400, Melanie Plageman wrote:
> >> On Fri, Mar 29, 2024 at 12:05:15PM +0100, Tomas Vondra wrote:
> >>>
> >>>
> >>> On 3/29/24 02:12, Thomas Munro wrote:
> >>>> On Fri, Mar 29, 2024 at 10:43 AM Tomas Vondra
> >>>> <[email protected]> wrote:
> >>>>> I think there's some sort of bug, triggering this assert in heapam
> >>>>>
> >>>>> Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno);
> >>>>
> >>>> Thanks for the repro. I can't seem to reproduce it (still trying) but
> >>>> I assume this is with Melanie's v11 patch set which had
> >>>> v11-0016-v10-Read-Stream-API.patch.
> >>>>
> >>>> Would you mind removing that commit and instead applying the v13
> >>>> stream_read.c patches[1]? v10 stream_read.c was a little confused
> >>>> about random I/O combining, which I fixed with a small adjustment to
> >>>> the conditions for the "if" statement right at the end of
> >>>> read_stream_look_ahead(). Sorry about that. The fixed version, with
> >>>> eic=4, with your test query using WHERE a < a, ends its scan with:
> >>>>
> >>>
> >>> I'll give that a try. Unfortunately unfortunately the v11 still has the
> >>> problem I reported about a week ago:
> >>>
> >>> ERROR: prefetch and main iterators are out of sync
> >>>
> >>> So I can't run the full benchmarks :-( but master vs. streaming read API
> >>> should work, I think.
> >>
> >> Odd, I didn't notice you reporting this ERROR popping up. Now that I
> >> take a look, v11 (at least, maybe also v10) had this very sill mistake:
> >>
> >> if (scan->bm_parallel == NULL &&
> >> scan->rs_pf_bhs_iterator &&
> >> hscan->pfblockno > hscan->rs_base.blockno)
> >> elog(ERROR, "prefetch and main iterators are out of sync");
> >>
> >> It errors out if the prefetch block is ahead of the current block --
> >> which is the opposite of what we want. I've fixed this in attached v12.
> >>
> >> This version also has v13 of the streaming read API. I noticed one
> >> mistake in my bitmapheap scan streaming read user -- it freed the
> >> streaming read object at the wrong time. I don't know if this was
> >> causing any other issues, but it at least is fixed in this version.
> >
> > Attached v13 is rebased over master (which includes the streaming read
> > API now). I also reset the streaming read object on rescan instead of
> > creating a new one each time.
> >
> > I don't know how much chance any of this has of going in to 17 now, but
> > I thought I would start looking into the regression repro Tomas provided
> > in [1].
> >
>
> My personal opinion is that we should try to get in as many of the the
> refactoring patches as possible, but I think it's probably too late for
> the actual switch to the streaming API.
Cool. In the attached v15, I have dropped all commits that are related
to the streaming read API and included *only* commits that are
beneficial to master. A few of the commits are merged or reordered as
well.
While going through the commits with this new goal in mind (forget about
the streaming read API for now), I realized that it doesn't make much
sense to just eliminate the layering violation for the current block and
leave it there for the prefetch block. I had de-prioritized solving this
when I thought we would just delete the prefetch code and replace it
with the streaming read.
Now that we aren't doing that, I've spent the day trying to resolve the
issues with pushing the prefetch code into heapam.c that I cited in [1].
0010 - 0013 are the result of this. They are not very polished yet and
need more cleanup and review (especially 0011, which is probably too
large), but I am happy with the solution I came up with.
Basically, there are too many members needed for bitmap heap scan to put
them all in the HeapScanDescData (don't want to bloat it). So, I've made
a new BitmapHeapScanDescData and associated begin/rescan/end() functions
In the end, with all patches applied, BitmapHeapNext() loops invoking
table_scan_bitmap_next_tuple() and table AMs can implement that however
they choose.
> > I'm also not sure if I should try and group the commits into fewer
> > commits now or wait until I have some idea of whether or not the
> > approach in 0013 and 0014 is worth pursuing.
> >
>
> You mean whether to pursue the approach in general, or for v17? I think
> it looks like the right approach, but for v17 see above :-(
>
> As for merging, I wouldn't do that. I looked at the commits and while
> some of them seem somewhat "trivial", I really like how you organized
> the commits, and kept those that just "move" code around, and those that
> actually change stuff. It's much easier to understand, IMO.
>
> I went through the first ~10 commits, and added some review - either as
> a separate commit, when possible, in the code as XXX comment, and also
> in the commit message. The code tweaks are utterly trivial (whitespace
> or indentation to make the line shorter). It shouldn't take much time to
> deal with those, I think.
Attached v15 incorporates your v14-0002-review.
For your v14-0008-review, I actually ended up removing that commit
because once I removed everything that was for streaming read API, it
became redundant with another commit.
For your v14-0010-review, we actually can't easily get rid of those
local variables because we make the iterators before we make the scan
descriptors and the commit following that commit moves the iterators
from the BitmapHeapScanState to the scan descriptor.
> I think the main focus should be updating the commit messages. If it was
> only a single patch, I'd probably try to write the messages myself, but
> with this many patches it'd be great if you could update those and I'll
> review that before commit.
I did my best to update the commit messages to be less specific and more
focused on "why should I care". I found myself wanting to explain why I
implemented something the way I did and then getting back into the
implementation details again. I'm not sure if I suceeded in having less
details and more substance.
> I always struggle with writing commit messages myself, and it takes me
> ages to write a good one (well, I think the message is good, but who
> knows ...). But I think a good message should be concise enough to
> explain what and why it's done. It may reference a thread for all the
> gory details, but the basic reasoning should be in the commit message.
> For example the message for "BitmapPrefetch use prefetch block recheck
> for skip fetch" now says that it "makes more sense to do X" but does not
> really say why that's the case. The linked message does, but it'd be
> good to have that in the message (because how would I know how much of
> the thread to read?).
I fixed that particular one. I tried to take that feedback and apply it
to other commit messages. I don't know how successful I was...
> Also, it'd be very helpful if you could update the author & reviewed-by
> fields. I'll review those before commit, ofc, but I admit I lost track
> of who reviewed which part.
I have updated reviewers. I didn't add reviewers on the ones that
haven't really been reviewed yet.
> I'd focus on the first ~8-9 commits or so for now, we can commit more if
> things go reasonably well.
Sounds good. I will spend cleanup time on 0010-0013 tomorrow but would
love to know if you agree with the direction before I spend more time.
- Melanie
[1] https://www.postgresql.org/message-id/20240323002211.on5vb5ulk6lsdb2u%40liskov
Attachments:
[text/x-diff] v15-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch (2.4K, ../20240405080634.d7c23f46xjhhxf5q@liskov/2-v15-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch)
download | inline diff:
From adae81352723f22ba2528dc4bf5f2216c7641428 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 18:50:29 -0500
Subject: [PATCH v15 01/13] BitmapHeapScan begin scan after bitmap creation
It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and then begin the scan on the underlying table.
This is primarily a cosmetic change for now, but later commits will pass
parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..c8c466e3c5 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
#endif /* USE_PREFETCH */
}
+
+ /*
+ * If this is the first scan of the underlying table, create the table
+ * scan descriptor and begin the scan.
+ */
+ if (!scan)
+ {
+ scan = table_beginscan_bm(node->ss.ss_currentRelation,
+ node->ss.ps.state->es_snapshot,
+ 0,
+ NULL);
+
+ node->ss.ss_currentScanDesc = scan;
+ }
+
node->initialized = true;
}
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
PlanState *outerPlan = outerPlanState(node);
/* rescan to release any page pin */
- table_rescan(node->ss.ss_currentScanDesc, NULL);
+ if (node->ss.ss_currentScanDesc)
+ table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* close heap scan
*/
- table_endscan(scanDesc);
+ if (scanDesc)
+ table_endscan(scanDesc);
+
}
/* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ss_currentRelation = currentRelation;
- scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
- estate->es_snapshot,
- 0,
- NULL);
-
/*
* all done.
*/
--
2.40.1
[text/x-diff] v15-0002-BitmapHeapScan-set-can_skip_fetch-later.patch (2.3K, ../20240405080634.d7c23f46xjhhxf5q@liskov/3-v15-0002-BitmapHeapScan-set-can_skip_fetch-later.patch)
download | inline diff:
From 6957ee25cf85784906a1315d148180858aedaad2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 13 Feb 2024 14:38:41 -0500
Subject: [PATCH v15 02/13] BitmapHeapScan set can_skip_fetch later
Set BitmapHeapScanState->can_skip_fetch in BitmapHeapNext() instead of
in ExecInitBitmapHeapScan(). This is a preliminary step to pushing the
skip fetch optimization into heap AM code.
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c8c466e3c5..2148a21531 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -105,6 +105,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ /*
+ * We can potentially skip fetching heap pages if we do not need any
+ * columns of the table, either for checking non-indexable quals or
+ * for returning data. This test is a bit simplistic, as it checks
+ * the stronger condition that there's no qual or return tlist at all.
+ * But in most cases it's probably not worth working harder than that.
+ */
+ node->can_skip_fetch = (node->ss.ps.plan->qual == NIL &&
+ node->ss.ps.plan->targetlist == NIL);
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -743,16 +753,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
-
- /*
- * We can potentially skip fetching heap pages if we do not need any
- * columns of the table, either for checking non-indexable quals or for
- * returning data. This test is a bit simplistic, as it checks the
- * stronger condition that there's no qual or return tlist at all. But in
- * most cases it's probably not worth working harder than that.
- */
- scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&
- node->scan.plan.targetlist == NIL);
+ scanstate->can_skip_fetch = false;
/*
* Miscellaneous initialization
--
2.40.1
[text/x-diff] v15-0003-Push-BitmapHeapScan-skip-fetch-optimization-into.patch (14.7K, ../20240405080634.d7c23f46xjhhxf5q@liskov/4-v15-0003-Push-BitmapHeapScan-skip-fetch-optimization-into.patch)
download | inline diff:
From 7566751f0ce9d68154e1f5616f7585334f497716 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 15:34:25 +0200
Subject: [PATCH v15 03/13] Push BitmapHeapScan skip fetch optimization into
table AM
7c70996ebf0949b142 introduced an optimization to allow bitmap table
scans to skip fetching a block from the heap if none of the underlying
data was needed and the block is marked all visible in the visibility
map. With the addition of table AMs, a FIXME was added to this code
indicating that it should be pushed into table AM-specific code, as not
all table AMs may use a visibility map in the same way.
Resolve this FIXME for the current block. The layering violation is
still present in BitmapHeapScans's prefetching code, which uses the
visibility map to decide whether or not to prefetch a block. However,
this will be fixed in an upcoming commit.
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 14 +++
src/backend/access/heap/heapam_handler.c | 29 +++++
src/backend/executor/nodeBitmapHeapscan.c | 124 +++++++---------------
src/include/access/heapam.h | 10 ++
src/include/access/tableam.h | 11 +-
src/include/nodes/execnodes.h | 8 +-
6 files changed, 102 insertions(+), 94 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index dada2ecd1e..10f2faaa60 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -967,6 +967,8 @@ heap_beginscan(Relation relation, Snapshot snapshot,
scan->rs_base.rs_flags = flags;
scan->rs_base.rs_parallel = parallel_scan;
scan->rs_strategy = NULL; /* set in initscan */
+ scan->rs_vmbuffer = InvalidBuffer;
+ scan->rs_empty_tuples_pending = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1055,6 +1057,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ if (BufferIsValid(scan->rs_vmbuffer))
+ {
+ ReleaseBuffer(scan->rs_vmbuffer);
+ scan->rs_vmbuffer = InvalidBuffer;
+ }
+
/*
* reinitialize scan descriptor
*/
@@ -1074,6 +1082,12 @@ heap_endscan(TableScanDesc sscan)
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ if (BufferIsValid(scan->rs_vmbuffer))
+ {
+ ReleaseBuffer(scan->rs_vmbuffer);
+ scan->rs_vmbuffer = InvalidBuffer;
+ }
+
/*
* decrement relation reference count and free scan descriptor storage
*/
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 3e7a6b5548..929b2cf8e7 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -27,6 +27,7 @@
#include "access/syncscan.h"
#include "access/tableam.h"
#include "access/tsmapi.h"
+#include "access/visibilitymap.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/index.h"
@@ -2198,6 +2199,24 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
+ /*
+ * We can skip fetching the heap page if we don't need any fields from the
+ * heap, the bitmap entries don't need rechecking, and all tuples on the
+ * page are visible to our transaction.
+ */
+ if (!(scan->rs_flags & SO_NEED_TUPLE) &&
+ !tbmres->recheck &&
+ VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &hscan->rs_vmbuffer))
+ {
+ /* can't be lossy in the skip_fetch case */
+ Assert(tbmres->ntuples >= 0);
+ Assert(hscan->rs_empty_tuples_pending >= 0);
+
+ hscan->rs_empty_tuples_pending += tbmres->ntuples;
+
+ return true;
+ }
+
/*
* Ignore any claimed entries past what we think is the end of the
* relation. It may have been extended after the start of our scan (we
@@ -2310,6 +2329,16 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
Page page;
ItemId lp;
+ if (hscan->rs_empty_tuples_pending > 0)
+ {
+ /*
+ * If we don't have to fetch the tuple, just return nulls.
+ */
+ ExecStoreAllNullTuple(slot);
+ hscan->rs_empty_tuples_pending--;
+ return true;
+ }
+
/*
* Out of range? If so, nothing more to look at on this page
*/
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 2148a21531..a8bc5dec53 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -105,16 +105,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
- /*
- * We can potentially skip fetching heap pages if we do not need any
- * columns of the table, either for checking non-indexable quals or
- * for returning data. This test is a bit simplistic, as it checks
- * the stronger condition that there's no qual or return tlist at all.
- * But in most cases it's probably not worth working harder than that.
- */
- node->can_skip_fetch = (node->ss.ps.plan->qual == NIL &&
- node->ss.ps.plan->targetlist == NIL);
-
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -195,10 +185,24 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!scan)
{
+ uint32 extra_flags = 0;
+
+ /*
+ * We can potentially skip fetching heap pages if we do not need
+ * any columns of the table, either for checking non-indexable
+ * quals or for returning data. This test is a bit simplistic, as
+ * it checks the stronger condition that there's no qual or return
+ * tlist at all. But in most cases it's probably not worth working
+ * harder than that.
+ */
+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)
+ extra_flags |= SO_NEED_TUPLE;
+
scan = table_beginscan_bm(node->ss.ss_currentRelation,
node->ss.ps.state->es_snapshot,
0,
- NULL);
+ NULL,
+ extra_flags);
node->ss.ss_currentScanDesc = scan;
}
@@ -208,7 +212,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool skip_fetch;
+ bool valid;
CHECK_FOR_INTERRUPTS();
@@ -229,37 +233,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres);
+
if (tbmres->ntuples >= 0)
node->exact_pages++;
else
node->lossy_pages++;
- /*
- * We can skip fetching the heap page if we don't need any fields
- * from the heap, and the bitmap entries don't need rechecking,
- * and all tuples on the page are visible to our transaction.
- *
- * XXX: It's a layering violation that we do these checks above
- * tableam, they should probably moved below it at some point.
- */
- skip_fetch = (node->can_skip_fetch &&
- !tbmres->recheck &&
- VM_ALL_VISIBLE(node->ss.ss_currentRelation,
- tbmres->blockno,
- &node->vmbuffer));
-
- if (skip_fetch)
- {
- /* can't be lossy in the skip_fetch case */
- Assert(tbmres->ntuples >= 0);
-
- /*
- * The number of tuples on this page is put into
- * node->return_empty_tuples.
- */
- node->return_empty_tuples = tbmres->ntuples;
- }
- else if (!table_scan_bitmap_next_block(scan, tbmres))
+ if (!valid)
{
/* AM doesn't think this block is valid, skip */
continue;
@@ -302,52 +283,33 @@ BitmapHeapNext(BitmapHeapScanState *node)
* should happen only when we have determined there is still something
* to do on the current page, else we may uselessly prefetch the same
* page we are just about to request for real.
- *
- * XXX: It's a layering violation that we do these checks above
- * tableam, they should probably moved below it at some point.
*/
BitmapPrefetch(node, scan);
- if (node->return_empty_tuples > 0)
+ /*
+ * Attempt to fetch tuple from AM.
+ */
+ if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
{
- /*
- * If we don't have to fetch the tuple, just return nulls.
- */
- ExecStoreAllNullTuple(slot);
-
- if (--node->return_empty_tuples == 0)
- {
- /* no more tuples to return in the next round */
- node->tbmres = tbmres = NULL;
- }
+ /* nothing more to look at on this page */
+ node->tbmres = tbmres = NULL;
+ continue;
}
- else
+
+ /*
+ * If we are using lossy info, we have to recheck the qual conditions
+ * at every tuple.
+ */
+ if (tbmres->recheck)
{
- /*
- * Attempt to fetch tuple from AM.
- */
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ econtext->ecxt_scantuple = slot;
+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))
{
- /* nothing more to look at on this page */
- node->tbmres = tbmres = NULL;
+ /* Fails recheck, so drop it and loop back for another */
+ InstrCountFiltered2(node, 1);
+ ExecClearTuple(slot);
continue;
}
-
- /*
- * If we are using lossy info, we have to recheck the qual
- * conditions at every tuple.
- */
- if (tbmres->recheck)
- {
- econtext->ecxt_scantuple = slot;
- if (!ExecQualAndReset(node->bitmapqualorig, econtext))
- {
- /* Fails recheck, so drop it and loop back for another */
- InstrCountFiltered2(node, 1);
- ExecClearTuple(slot);
- continue;
- }
- }
}
/* OK to return this tuple */
@@ -519,7 +481,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* it did for the current heap page; which is not a certainty
* but is true in many cases.
*/
- skip_fetch = (node->can_skip_fetch &&
+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
(node->tbmres ? !node->tbmres->recheck : false) &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
@@ -570,7 +532,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
}
/* As above, skip prefetch if we expect not to need page */
- skip_fetch = (node->can_skip_fetch &&
+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
(node->tbmres ? !node->tbmres->recheck : false) &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
@@ -640,8 +602,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->vmbuffer != InvalidBuffer)
- ReleaseBuffer(node->vmbuffer);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
@@ -651,7 +611,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
node->initialized = false;
node->shared_tbmiterator = NULL;
node->shared_prefetch_iterator = NULL;
- node->vmbuffer = InvalidBuffer;
node->pvmbuffer = InvalidBuffer;
ExecScanReScan(&node->ss);
@@ -696,8 +655,6 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
- if (node->vmbuffer != InvalidBuffer)
- ReleaseBuffer(node->vmbuffer);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
@@ -741,8 +698,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->tbm = NULL;
scanstate->tbmiterator = NULL;
scanstate->tbmres = NULL;
- scanstate->return_empty_tuples = 0;
- scanstate->vmbuffer = InvalidBuffer;
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
@@ -753,7 +708,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
- scanstate->can_skip_fetch = false;
/*
* Miscellaneous initialization
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 2765efc4e5..750ea30852 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -76,6 +76,16 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /*
+ * These fields are only used for bitmap scans for the "skip fetch"
+ * optimization. Bitmap scans needing no fields from the heap may skip
+ * fetching an all visible block, instead using the number of tuples per
+ * block reported by the bitmap to determine how many NULL-filled tuples
+ * to return.
+ */
+ Buffer rs_vmbuffer;
+ int rs_empty_tuples_pending;
+
/* these fields only used in page-at-a-time mode and for bitmap scans */
int rs_cindex; /* current tuple's index in vistuples */
int rs_ntuples; /* number of visible tuples on page */
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index e7eeb75409..55f1139757 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -63,6 +63,13 @@ typedef enum ScanOptions
/* unregister snapshot at scan end? */
SO_TEMP_SNAPSHOT = 1 << 9,
+
+ /*
+ * At the discretion of the table AM, bitmap table scans may be able to
+ * skip fetching a block from the table if none of the table data is
+ * needed. If table data may be needed, set SO_NEED_TUPLE.
+ */
+ SO_NEED_TUPLE = 1 << 10,
} ScanOptions;
/*
@@ -937,9 +944,9 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key)
+ int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
- uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+ uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
}
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e57ebd7288..fa2f70b7a4 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1794,10 +1794,7 @@ typedef struct ParallelBitmapHeapState
* tbm bitmap obtained from child index scan(s)
* tbmiterator iterator for scanning current pages
* tbmres current-page data
- * can_skip_fetch can we potentially skip tuple fetches in this scan?
- * return_empty_tuples number of empty tuples to return
- * vmbuffer buffer for visibility-map lookups
- * pvmbuffer ditto, for prefetched pages
+ * 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
* prefetch_iterator iterator for prefetching ahead of current page
@@ -1817,9 +1814,6 @@ typedef struct BitmapHeapScanState
TIDBitmap *tbm;
TBMIterator *tbmiterator;
TBMIterateResult *tbmres;
- bool can_skip_fetch;
- int return_empty_tuples;
- Buffer vmbuffer;
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
--
2.40.1
[text/x-diff] v15-0004-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch (2.4K, ../20240405080634.d7c23f46xjhhxf5q@liskov/5-v15-0004-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch)
download | inline diff:
From 9d18888d6e80bc7e2bb676b2803a463f5ee4b04c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 19:03:24 -0500
Subject: [PATCH v15 04/13] BitmapPrefetch use prefetch block recheck for skip
fetch
As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for
the current block to determine whether or not it could skip prefetching
the proposed prefetch block. This doesn't seem right. We should use the
prefetch block's recheck flag to determine whether or not to prefetch
it. The current block's recheck flag has no relationship to the prefetch
block's recheck flag. See this [1] thread on hackers reporting the
issue.
[1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.gmail.com
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a8bc5dec53..2e2cec8b3b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* skip this prefetch call, but continue to run the prefetch
* logic normally. (Would it be better not to increment
* prefetch_pages?)
- *
- * This depends on the assumption that the index AM will
- * report the same recheck flag for this future heap page as
- * it did for the current heap page; which is not a certainty
- * but is true in many cases.
*/
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- (node->tbmres ? !node->tbmres->recheck : false) &&
+ !tbmpre->recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
&node->pvmbuffer));
@@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
/* As above, skip prefetch if we expect not to need page */
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- (node->tbmres ? !node->tbmres->recheck : false) &&
+ !tbmpre->recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
&node->pvmbuffer));
--
2.40.1
[text/x-diff] v15-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch (2.5K, ../20240405080634.d7c23f46xjhhxf5q@liskov/6-v15-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch)
download | inline diff:
From 200f5b4e4e79d9cd9e0d8f02df574d616a3569eb Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 19:04:48 -0500
Subject: [PATCH v15 05/13] Update BitmapAdjustPrefetchIterator parameter type
to BlockNumber
BitmapAdjustPrefetchIterator() only used the blockno member of the
passed in TBMIterateResult to ensure that the prefetch iterator and
regular iterator stay in sync. Pass it the BlockNumber only. This will
allow us to move away from using the TBMIterateResult outside of table
AM specific code.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 2e2cec8b3b..795a893035 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -52,7 +52,7 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- TBMIterateResult *tbmres);
+ BlockNumber blockno);
static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
@@ -231,7 +231,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
break;
}
- BitmapAdjustPrefetchIterator(node, tbmres);
+ BitmapAdjustPrefetchIterator(node, tbmres->blockno);
valid = table_scan_bitmap_next_block(scan, tbmres);
@@ -342,7 +342,7 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
*/
static inline void
BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- TBMIterateResult *tbmres)
+ BlockNumber blockno)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
@@ -361,7 +361,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
/* Do not let the prefetch iterator get behind the main one */
TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);
- if (tbmpre == NULL || tbmpre->blockno != tbmres->blockno)
+ if (tbmpre == NULL || tbmpre->blockno != blockno)
elog(ERROR, "prefetch and main iterators are out of sync");
}
return;
--
2.40.1
[text/x-diff] v15-0006-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch (3.1K, ../20240405080634.d7c23f46xjhhxf5q@liskov/7-v15-0006-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch)
download | inline diff:
From 001b361b38b294b8cbf7d7c4487804d56f0929e1 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 13 Feb 2024 10:17:47 -0500
Subject: [PATCH v15 06/13] Reduce scope of BitmapHeapScan tbmiterator local
variables
A future patch will change where tbmiterators are initialized and saved.
To simplify that diff, move them into a tighter scope.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 795a893035..8403be84f3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -207,6 +203,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -222,9 +220,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
[text/x-diff] v15-0007-table_scan_bitmap_next_block-counts-lossy-and-ex.patch (5.1K, ../20240405080634.d7c23f46xjhhxf5q@liskov/8-v15-0007-table_scan_bitmap_next_block-counts-lossy-and-ex.patch)
download | inline diff:
From b865213848634d95f22ce50aa701eb1ac4722c5c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 26 Feb 2024 20:34:07 -0500
Subject: [PATCH v15 07/13] table_scan_bitmap_next_block counts lossy and exact
pages
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So the table AM must keep
track of whether or not individual TBMIterateResult's blocks were
represented lossily in the bitmap for the purposes of EXPLAIN counters.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 8 +++++++-
src/backend/executor/nodeBitmapHeapscan.c | 13 +++----------
src/include/access/tableam.h | 22 ++++++++++++++++------
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 929b2cf8e7..b5ab104ec2 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2188,7 +2188,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)
+ TBMIterateResult *tbmres,
+ long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2316,6 +2317,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ if (tbmres->ntuples < 0)
+ (*lossy_pages)++;
+ else
+ (*exact_pages)++;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 8403be84f3..68f9ded168 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -210,8 +210,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
-
CHECK_FOR_INTERRUPTS();
/*
@@ -231,19 +229,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
-
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
- node->lossy_pages++;
-
- if (!valid)
+ if (!table_scan_bitmap_next_block(scan, tbmres,
+ &node->lossy_pages, &node->exact_pages))
{
/* AM doesn't think this block is valid, skip */
continue;
}
+
/* Adjust the prefetch target */
BitmapAdjustPrefetchTarget(node);
}
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 55f1139757..e2ad8d0728 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -789,6 +789,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy_pages is incremented if the bitmap is lossy for the selected
+ * block; otherwise, exact_pages is incremented.
+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -804,7 +807,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);
+ struct TBMIterateResult *tbmres,
+ long *lossy_pages, long *exact_pages);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1968,17 +1972,21 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of
- * a bitmap table scan. `scan` needs to have been started via
+ * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of a
+ * bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.
+ * the page, true otherwise. lossy_pages is incremented is the block's
+ * representation in the bitmap is lossy; otherwise, exact_pages is
+ * incremented.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)
+ struct TBMIterateResult *tbmres,
+ long *lossy_pages,
+ long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1989,7 +1997,9 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);
+ tbmres,
+ lossy_pages,
+ exact_pages);
}
/*
--
2.40.1
[text/x-diff] v15-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch (4.1K, ../20240405080634.d7c23f46xjhhxf5q@liskov/9-v15-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch)
download | inline diff:
From c4098d4d1ec206b837711b03f5dfc322346be497 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 18:13:41 -0500
Subject: [PATCH v15 08/13] Remove table_scan_bitmap_next_tuple parameter
tbmres
Future commits will push all of the logic for choosing the next block
into the table AM specific code. The table AM will be responsible for
iterating and reading in the right blocks.
Thus, it no longer makes sense to use the TBMIterateResult (which
contains a block number) as a means of communication between
table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index b5ab104ec2..efd1a66a09 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2327,7 +2327,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 68f9ded168..951a98c101 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -280,7 +280,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index e2ad8d0728..2efa97f602 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -780,10 +780,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some
- * AMs it will make more sense to do all the work referencing `tbmres`
- * contents here, for others it might be better to defer more work to
- * scan_bitmap_next_tuple.
+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -814,15 +811,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing
- * `tbmres` contents in scan_bitmap_next_block, for others it might be
- * better to defer more work to this callback.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2012,7 +2004,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2024,7 +2015,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
[text/x-diff] v15-0009-Make-table_scan_bitmap_next_block-async-friendly.patch (22.4K, ../20240405080634.d7c23f46xjhhxf5q@liskov/10-v15-0009-Make-table_scan_bitmap_next_block-async-friendly.patch)
download | inline diff:
From 520acaf3df90cec9db518f6fda660ae9bb81fba0 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 14 Mar 2024 12:39:28 -0400
Subject: [PATCH v15 09/13] Make table_scan_bitmap_next_block() async friendly
table_scan_bitmap_next_block() previously returned false if we did not
wish to call table_scan_bitmap_next_tuple() on the tuples on the page.
This could happen when there were no visible tuples on the page or, due
to concurrent activity on the table, the block returned by the iterator
is past the end of the table recorded when the scan started.
This forced the caller to be responsible for determining if additional
blocks should be fetched and then for invoking
table_scan_bitmap_next_block() for these blocks.
It makes more sense for table_scan_bitmap_next_block() to be responsible
for finding a block that is not past the end of the table (as of the
time that the scan began) and for table_scan_bitmap_next_tuple() to
return false if there are no visible tuples on the page.
This also allows us to move responsibility for the iterator to table AM
specific code. This means handling invalid blocks is entirely up to the
table AM.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 58 +++++--
src/backend/executor/nodeBitmapHeapscan.c | 186 ++++++++++------------
src/include/access/relscan.h | 7 +
src/include/access/tableam.h | 68 +++++---
src/include/nodes/execnodes.h | 12 +-
5 files changed, 190 insertions(+), 141 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index efd1a66a09..9d3e7c7fda 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2188,18 +2188,52 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres,
+ BlockNumber *blockno, bool *recheck,
long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
- BlockNumber block = tbmres->blockno;
+ BlockNumber block;
Buffer buffer;
Snapshot snapshot;
int ntup;
+ TBMIterateResult *tbmres;
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
+ *blockno = InvalidBlockNumber;
+ *recheck = true;
+
+ do
+ {
+ CHECK_FOR_INTERRUPTS();
+
+ if (scan->shared_tbmiterator)
+ tbmres = tbm_shared_iterate(scan->shared_tbmiterator);
+ else
+ tbmres = tbm_iterate(scan->tbmiterator);
+
+ if (tbmres == NULL)
+ {
+ /* no more entries in the bitmap */
+ Assert(hscan->rs_empty_tuples_pending == 0);
+ return false;
+ }
+
+ /*
+ * Ignore any claimed entries past what we think is the end of the
+ * relation. It may have been extended after the start of our scan (we
+ * only hold an AccessShareLock, and it could be inserts from this
+ * backend). We don't take this optimization in SERIALIZABLE
+ * isolation though, as we need to examine all invisible tuples
+ * reachable by the index.
+ */
+ } while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks);
+
+ /* Got a valid block */
+ *blockno = tbmres->blockno;
+ *recheck = tbmres->recheck;
+
/*
* We can skip fetching the heap page if we don't need any fields from the
* heap, the bitmap entries don't need rechecking, and all tuples on the
@@ -2218,16 +2252,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
return true;
}
- /*
- * Ignore any claimed entries past what we think is the end of the
- * relation. It may have been extended after the start of our scan (we
- * only hold an AccessShareLock, and it could be inserts from this
- * backend). We don't take this optimization in SERIALIZABLE isolation
- * though, as we need to examine all invisible tuples reachable by the
- * index.
- */
- if (!IsolationIsSerializable() && block >= hscan->rs_nblocks)
- return false;
+ block = tbmres->blockno;
/*
* Acquire pin on the target heap page, trading in any pin we held before.
@@ -2322,7 +2347,14 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
else
(*exact_pages)++;
- return ntup > 0;
+ /*
+ * Return true to indicate that a valid block was found and the bitmap is
+ * not exhausted. If there are no visible tuples on this page,
+ * hscan->rs_ntuples will be 0 and heapam_scan_bitmap_next_tuple() will
+ * return false returning control to this function to advance to the next
+ * block in the bitmap.
+ */
+ return true;
}
static bool
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 951a98c101..e1b13ddaa6 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -51,8 +51,7 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
-static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- BlockNumber blockno);
+static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);
static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
@@ -71,7 +70,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
@@ -83,7 +81,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- tbmres = node->tbmres;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
@@ -111,7 +108,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->tbm = tbm;
tbmiterator = tbm_begin_iterate(tbm);
- node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
@@ -164,7 +160,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
/* Allocate a private iterator and attach the shared state to it */
shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
- node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
@@ -203,48 +198,19 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- node->tbmiterator = tbmiterator;
- node->shared_tbmiterator = shared_tbmiterator;
+ scan->tbmiterator = tbmiterator;
+ scan->shared_tbmiterator = shared_tbmiterator;
+
node->initialized = true;
+
+ goto new_page;
}
for (;;)
{
- CHECK_FOR_INTERRUPTS();
-
- /*
- * Get next page of results if needed
- */
- if (tbmres == NULL)
- {
- if (!pstate)
- node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
- else
- node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
- if (tbmres == NULL)
- {
- /* no more entries in the bitmap */
- break;
- }
-
- BitmapAdjustPrefetchIterator(node, tbmres->blockno);
-
- if (!table_scan_bitmap_next_block(scan, tbmres,
- &node->lossy_pages, &node->exact_pages))
- {
- /* AM doesn't think this block is valid, skip */
- continue;
- }
-
-
- /* Adjust the prefetch target */
- BitmapAdjustPrefetchTarget(node);
- }
- else
+ while (table_scan_bitmap_next_tuple(scan, slot))
{
- /*
- * Continuing in previously obtained page.
- */
+ CHECK_FOR_INTERRUPTS();
#ifdef USE_PREFETCH
@@ -266,45 +232,56 @@ BitmapHeapNext(BitmapHeapScanState *node)
SpinLockRelease(&pstate->mutex);
}
#endif /* USE_PREFETCH */
- }
- /*
- * We issue prefetch requests *after* fetching the current page to try
- * to avoid having prefetching interfere with the main I/O. Also, this
- * should happen only when we have determined there is still something
- * to do on the current page, else we may uselessly prefetch the same
- * page we are just about to request for real.
- */
- BitmapPrefetch(node, scan);
+ /*
+ * We issue prefetch requests *after* fetching the current page to
+ * try to avoid having prefetching interfere with the main I/O.
+ * Also, this should happen only when we have determined there is
+ * still something to do on the current page, else we may
+ * uselessly prefetch the same page we are just about to request
+ * for real.
+ */
+ BitmapPrefetch(node, scan);
- /*
- * Attempt to fetch tuple from AM.
- */
- if (!table_scan_bitmap_next_tuple(scan, slot))
- {
- /* nothing more to look at on this page */
- node->tbmres = tbmres = NULL;
- continue;
+ /*
+ * If we are using lossy info, we have to recheck the qual
+ * conditions at every tuple.
+ */
+ if (node->recheck)
+ {
+ econtext->ecxt_scantuple = slot;
+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))
+ {
+ /* Fails recheck, so drop it and loop back for another */
+ InstrCountFiltered2(node, 1);
+ ExecClearTuple(slot);
+ continue;
+ }
+ }
+
+ /* OK to return this tuple */
+ return slot;
}
+new_page:
+
+ BitmapAdjustPrefetchIterator(node);
+
+ if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck,
+ &node->lossy_pages, &node->exact_pages))
+ break;
+
/*
- * If we are using lossy info, we have to recheck the qual conditions
- * at every tuple.
+ * If serial, we can error out if the the prefetch block doesn't stay
+ * ahead of the current block.
*/
- if (tbmres->recheck)
- {
- econtext->ecxt_scantuple = slot;
- if (!ExecQualAndReset(node->bitmapqualorig, econtext))
- {
- /* Fails recheck, so drop it and loop back for another */
- InstrCountFiltered2(node, 1);
- ExecClearTuple(slot);
- continue;
- }
- }
+ if (node->pstate == NULL &&
+ node->prefetch_iterator &&
+ node->pfblockno < node->blockno)
+ elog(ERROR, "prefetch and main iterators are out of sync");
- /* OK to return this tuple */
- return slot;
+ /* Adjust the prefetch target */
+ BitmapAdjustPrefetchTarget(node);
}
/*
@@ -330,13 +307,17 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
/*
* BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
+ *
+ * We keep track of how far the prefetch iterator is ahead of the main
+ * iterator in prefetch_pages. For each block the main iterator returns, we
+ * decrement prefetch_pages.
*/
static inline void
-BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- BlockNumber blockno)
+BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ TBMIterateResult *tbmpre;
if (pstate == NULL)
{
@@ -350,14 +331,17 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
else if (prefetch_iterator)
{
/* Do not let the prefetch iterator get behind the main one */
- TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);
-
- if (tbmpre == NULL || tbmpre->blockno != blockno)
- elog(ERROR, "prefetch and main iterators are out of sync");
+ tbmpre = tbm_iterate(prefetch_iterator);
+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
return;
}
+ /*
+ * Adjusting the prefetch iterator before invoking
+ * table_scan_bitmap_next_block() keeps prefetch distance higher across
+ * the parallel workers.
+ */
if (node->prefetch_maximum > 0)
{
TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
@@ -382,7 +366,10 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
* case.
*/
if (prefetch_iterator)
- tbm_shared_iterate(prefetch_iterator);
+ {
+ tbmpre = tbm_shared_iterate(prefetch_iterator);
+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
+ }
}
}
#endif /* USE_PREFETCH */
@@ -460,6 +447,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
node->prefetch_pages++;
+ node->pfblockno = tbmpre->blockno;
/*
* If we expect not to have to actually read this heap page,
@@ -517,6 +505,8 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
+ node->pfblockno = tbmpre->blockno;
+
/* As above, skip prefetch if we expect not to need page */
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
!tbmpre->recheck &&
@@ -578,12 +568,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
- if (node->tbmiterator)
- tbm_end_iterate(node->tbmiterator);
if (node->prefetch_iterator)
tbm_end_iterate(node->prefetch_iterator);
- if (node->shared_tbmiterator)
- tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->tbm)
@@ -591,13 +577,13 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
- node->tbmiterator = NULL;
- node->tbmres = NULL;
node->prefetch_iterator = NULL;
node->initialized = false;
- node->shared_tbmiterator = NULL;
node->shared_prefetch_iterator = NULL;
node->pvmbuffer = InvalidBuffer;
+ node->recheck = true;
+ node->blockno = InvalidBlockNumber;
+ node->pfblockno = InvalidBlockNumber;
ExecScanReScan(&node->ss);
@@ -628,28 +614,24 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
*/
ExecEndNode(outerPlanState(node));
+
+ /*
+ * close heap scan
+ */
+ if (scanDesc)
+ table_endscan(scanDesc);
+
/*
* release bitmaps and buffers if any
*/
- if (node->tbmiterator)
- tbm_end_iterate(node->tbmiterator);
if (node->prefetch_iterator)
tbm_end_iterate(node->prefetch_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->shared_tbmiterator)
- tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
-
- /*
- * close heap scan
- */
- if (scanDesc)
- table_endscan(scanDesc);
-
}
/* ----------------------------------------------------------------
@@ -682,8 +664,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ps.ExecProcNode = ExecBitmapHeapScan;
scanstate->tbm = NULL;
- scanstate->tbmiterator = NULL;
- scanstate->tbmres = NULL;
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
@@ -691,9 +671,11 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->prefetch_pages = 0;
scanstate->prefetch_target = 0;
scanstate->initialized = false;
- scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
+ scanstate->recheck = true;
+ scanstate->blockno = InvalidBlockNumber;
+ scanstate->pfblockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index 521043304a..92b829cebc 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -24,6 +24,9 @@
struct ParallelTableScanDescData;
+struct TBMIterator;
+struct TBMSharedIterator;
+
/*
* Generic descriptor for table scans. This is the base-class for table scans,
* which needs to be embedded in the scans of individual AMs.
@@ -40,6 +43,10 @@ typedef struct TableScanDescData
ItemPointerData rs_mintid;
ItemPointerData rs_maxtid;
+ /* Only used for Bitmap table scans */
+ struct TBMIterator *tbmiterator;
+ struct TBMSharedIterator *shared_tbmiterator;
+
/*
* Information about type and behaviour of the scan, a bitmask of members
* of the ScanOptions enum (see tableam.h).
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 2efa97f602..42c67a128e 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -22,6 +22,7 @@
#include "access/xact.h"
#include "commands/vacuum.h"
#include "executor/tuptable.h"
+#include "nodes/tidbitmap.h"
#include "utils/rel.h"
#include "utils/snapshot.h"
@@ -773,19 +774,14 @@ typedef struct TableAmRoutine
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part
- * of a bitmap table scan. `scan` was started via table_beginscan_bm().
- * Return false if there are no tuples to be found on the page, true
- * otherwise.
+ * Prepare to fetch / check / return tuples from `blockno` as part of a
+ * bitmap table scan. `scan` was started via table_beginscan_bm(). Return
+ * false if the bitmap is exhausted and true otherwise.
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
* make sense to perform tuple visibility checks at this time).
*
- * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
- * on the page have to be returned, otherwise the tuples at offsets in
- * `tbmres->offsets` need to be returned.
- *
* lossy_pages is incremented if the bitmap is lossy for the selected
* block; otherwise, exact_pages is incremented.
*
@@ -804,7 +800,7 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
+ BlockNumber *blockno, bool *recheck,
long *lossy_pages, long *exact_pages);
/*
@@ -942,9 +938,13 @@ static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
+ TableScanDesc result;
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
+ result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
+ result->shared_tbmiterator = NULL;
+ result->tbmiterator = NULL;
+ return result;
}
/*
@@ -991,6 +991,21 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN)
+ {
+ if (scan->shared_tbmiterator)
+ {
+ tbm_end_shared_iterate(scan->shared_tbmiterator);
+ scan->shared_tbmiterator = NULL;
+ }
+
+ if (scan->tbmiterator)
+ {
+ tbm_end_iterate(scan->tbmiterator);
+ scan->tbmiterator = NULL;
+ }
+ }
+
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1001,6 +1016,21 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN)
+ {
+ if (scan->shared_tbmiterator)
+ {
+ tbm_end_shared_iterate(scan->shared_tbmiterator);
+ scan->shared_tbmiterator = NULL;
+ }
+
+ if (scan->tbmiterator)
+ {
+ tbm_end_iterate(scan->tbmiterator);
+ scan->tbmiterator = NULL;
+ }
+ }
+
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
@@ -1964,19 +1994,18 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of a
- * bitmap table scan. `scan` needs to have been started via
- * table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise. lossy_pages is incremented is the block's
- * representation in the bitmap is lossy; otherwise, exact_pages is
- * incremented.
+ * Prepare to fetch / check / return tuples as part of a bitmap table scan.
+ * `scan` needs to have been started via table_beginscan_bm(). Returns false if
+ * there are no more blocks in the bitmap, true otherwise. lossy_pages is
+ * incremented is the block's representation in the bitmap is lossy; otherwise,
+ * exact_pages is incremented.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
+ BlockNumber *blockno, bool *recheck,
long *lossy_pages,
long *exact_pages)
{
@@ -1989,9 +2018,8 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres,
- lossy_pages,
- exact_pages);
+ blockno, recheck,
+ lossy_pages, exact_pages);
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index fa2f70b7a4..d96703b04d 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1792,8 +1792,6 @@ typedef struct ParallelBitmapHeapState
*
* bitmapqualorig execution state for bitmapqualorig expressions
* tbm bitmap obtained from child index scan(s)
- * 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
@@ -1802,9 +1800,11 @@ typedef struct ParallelBitmapHeapState
* prefetch_target current target prefetch distance
* prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
- * shared_tbmiterator shared iterator
* shared_prefetch_iterator shared iterator for prefetching
* pstate shared state for parallel bitmap scan
+ * recheck do current page's tuples need recheck
+ * blockno used to validate pf and current block in sync
+ * pfblockno used to validate pf stays ahead of current block
* ----------------
*/
typedef struct BitmapHeapScanState
@@ -1812,8 +1812,6 @@ typedef struct BitmapHeapScanState
ScanState ss; /* its first field is NodeTag */
ExprState *bitmapqualorig;
TIDBitmap *tbm;
- TBMIterator *tbmiterator;
- TBMIterateResult *tbmres;
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
@@ -1822,9 +1820,11 @@ typedef struct BitmapHeapScanState
int prefetch_target;
int prefetch_maximum;
bool initialized;
- TBMSharedIterator *shared_tbmiterator;
TBMSharedIterator *shared_prefetch_iterator;
ParallelBitmapHeapState *pstate;
+ bool recheck;
+ BlockNumber blockno;
+ BlockNumber pfblockno;
} BitmapHeapScanState;
/* ----------------
--
2.40.1
[text/x-diff] v15-0010-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch (16.7K, ../20240405080634.d7c23f46xjhhxf5q@liskov/11-v15-0010-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch)
download | inline diff:
From dcdfa8174ef7927f64bfb07f29bf0e9c2a45fde7 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 25 Mar 2024 11:05:51 -0400
Subject: [PATCH v15 10/13] Unify parallel and serial BitmapHeapScan iterator
interfaces
Introduce a new type, BitmapHeapIterator, which allows unified access to both
TBMIterator and TBMSharedIterators. This encapsulates the parallel and serial
iterators and their access and makes the bitmap heap scan code a bit cleaner.
This naturally lends itself to a bit of reorganization of the
!node->initialized path in BitmapHeapNext().
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 5 +-
src/backend/executor/nodeBitmapHeapscan.c | 172 ++++++++++++----------
src/include/access/relscan.h | 7 +-
src/include/access/tableam.h | 40 +----
src/include/executor/nodeBitmapHeapscan.h | 7 +
src/include/nodes/execnodes.h | 13 +-
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 124 insertions(+), 121 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 9d3e7c7fda..5e57d19d91 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2208,10 +2208,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
{
CHECK_FOR_INTERRUPTS();
- if (scan->shared_tbmiterator)
- tbmres = tbm_shared_iterate(scan->shared_tbmiterator);
- else
- tbmres = tbm_iterate(scan->tbmiterator);
+ tbmres = bhs_iterate(&scan->rs_bhs_iterator);
if (tbmres == NULL)
{
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index e1b13ddaa6..70b560b8ee 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -57,6 +57,61 @@ static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
+/*
+ * Start iteration on a shared or non-shared bitmap iterator. Note that tbm
+ * will only be provided by serial BitmapHeapScan callers. dsa and dsp will
+ * only be provided by parallel BitmapHeapScan callers.
+ */
+void
+bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,
+ dsa_area *dsa, dsa_pointer dsp)
+{
+ Assert(iterator);
+
+ iterator->serial = NULL;
+ iterator->parallel = NULL;
+ iterator->exhausted = false;
+
+ /* Allocate a private iterator and attach the shared state to it */
+ if (DsaPointerIsValid(dsp))
+ iterator->parallel = tbm_attach_shared_iterate(dsa, dsp);
+ else
+ iterator->serial = tbm_begin_iterate(tbm);
+}
+
+/*
+ * Get the next TBMIterateResult from the bitmap iterator.
+ */
+TBMIterateResult *
+bhs_iterate(BitmapHeapIterator *iterator)
+{
+ Assert(iterator);
+ Assert(!iterator->exhausted);
+
+ if (iterator->serial)
+ return tbm_iterate(iterator->serial);
+ else
+ return tbm_shared_iterate(iterator->parallel);
+}
+
+/*
+ * Clean up the bitmap iterator.
+ */
+void
+bhs_end_iterate(BitmapHeapIterator *iterator)
+{
+ Assert(iterator);
+
+ if (iterator->serial)
+ tbm_end_iterate(iterator->serial);
+ else if (iterator->parallel)
+ tbm_end_shared_iterate(iterator->parallel);
+
+ iterator->serial = NULL;
+ iterator->parallel = NULL;
+ iterator->exhausted = true;
+}
+
/* ----------------------------------------------------------------
* BitmapHeapNext
@@ -96,43 +151,23 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
+ /*
+ * The leader will immediately come out of the function, but others
+ * will be blocked until leader populates the TBM and wakes them up.
+ */
+ bool init_shared_state = node->pstate ?
+ BitmapShouldInitializeSharedState(node->pstate) : false;
- if (!pstate)
+ if (!pstate || init_shared_state)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
if (!tbm || !IsA(tbm, TIDBitmap))
elog(ERROR, "unrecognized result from subplan");
-
node->tbm = tbm;
- tbmiterator = tbm_begin_iterate(tbm);
-#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
+ if (init_shared_state)
{
- node->prefetch_iterator = tbm_begin_iterate(tbm);
- node->prefetch_pages = 0;
- node->prefetch_target = -1;
- }
-#endif /* USE_PREFETCH */
- }
- else
- {
- /*
- * The leader will immediately come out of the function, but
- * others will be blocked until leader populates the TBM and wakes
- * them up.
- */
- if (BitmapShouldInitializeSharedState(pstate))
- {
- tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
- if (!tbm || !IsA(tbm, TIDBitmap))
- elog(ERROR, "unrecognized result from subplan");
-
- node->tbm = tbm;
-
/*
* Prepare to iterate over the TBM. This will return the
* dsa_pointer of the iterator state which will be used by
@@ -153,21 +188,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
pstate->prefetch_target = -1;
}
#endif
-
/* We have initialized the shared state so wake up others. */
BitmapDoneInitializingSharedState(pstate);
}
-
- /* Allocate a private iterator and attach the shared state to it */
- shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
-
-#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
- {
- node->shared_prefetch_iterator =
- tbm_attach_shared_iterate(dsa, pstate->prefetch_iterator);
- }
-#endif /* USE_PREFETCH */
}
/*
@@ -198,8 +221,23 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- scan->tbmiterator = tbmiterator;
- scan->shared_tbmiterator = shared_tbmiterator;
+ bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa,
+ pstate ?
+ pstate->tbmiterator :
+ InvalidDsaPointer);
+
+#ifdef USE_PREFETCH
+ if (node->prefetch_maximum > 0)
+ {
+ bhs_begin_iterate(&node->pf_iterator, tbm, dsa,
+ pstate ?
+ pstate->prefetch_iterator :
+ InvalidDsaPointer);
+ /* Only used for serial BHS */
+ node->prefetch_pages = 0;
+ node->prefetch_target = -1;
+ }
+#endif /* USE_PREFETCH */
node->initialized = true;
@@ -276,7 +314,7 @@ new_page:
* ahead of the current block.
*/
if (node->pstate == NULL &&
- node->prefetch_iterator &&
+ !node->pf_iterator.exhausted &&
node->pfblockno < node->blockno)
elog(ERROR, "prefetch and main iterators are out of sync");
@@ -317,21 +355,20 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
TBMIterateResult *tbmpre;
if (pstate == NULL)
{
- TBMIterator *prefetch_iterator = node->prefetch_iterator;
-
if (node->prefetch_pages > 0)
{
/* The main iterator has closed the distance by one page */
node->prefetch_pages--;
}
- else if (prefetch_iterator)
+ else if (!prefetch_iterator->exhausted)
{
/* Do not let the prefetch iterator get behind the main one */
- tbmpre = tbm_iterate(prefetch_iterator);
+ tbmpre = bhs_iterate(prefetch_iterator);
node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
return;
@@ -344,8 +381,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
*/
if (node->prefetch_maximum > 0)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
-
SpinLockAcquire(&pstate->mutex);
if (pstate->prefetch_pages > 0)
{
@@ -365,9 +400,9 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
* we don't validate the blockno here as we do in non-parallel
* case.
*/
- if (prefetch_iterator)
+ if (!prefetch_iterator->exhausted)
{
- tbmpre = tbm_shared_iterate(prefetch_iterator);
+ tbmpre = bhs_iterate(prefetch_iterator);
node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
}
@@ -427,23 +462,21 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
if (pstate == NULL)
{
- TBMIterator *prefetch_iterator = node->prefetch_iterator;
-
- if (prefetch_iterator)
+ if (!prefetch_iterator->exhausted)
{
while (node->prefetch_pages < node->prefetch_target)
{
- TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);
+ TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
bool skip_fetch;
if (tbmpre == NULL)
{
/* No more pages to prefetch */
- tbm_end_iterate(prefetch_iterator);
- node->prefetch_iterator = NULL;
+ bhs_end_iterate(prefetch_iterator);
break;
}
node->prefetch_pages++;
@@ -471,9 +504,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (pstate->prefetch_pages < pstate->prefetch_target)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
-
- if (prefetch_iterator)
+ if (!prefetch_iterator->exhausted)
{
while (1)
{
@@ -496,12 +527,11 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (!do_prefetch)
return;
- tbmpre = tbm_shared_iterate(prefetch_iterator);
+ tbmpre = bhs_iterate(prefetch_iterator);
if (tbmpre == NULL)
{
/* No more pages to prefetch */
- tbm_end_shared_iterate(prefetch_iterator);
- node->shared_prefetch_iterator = NULL;
+ bhs_end_iterate(prefetch_iterator);
break;
}
@@ -568,18 +598,14 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
- if (node->prefetch_iterator)
- tbm_end_iterate(node->prefetch_iterator);
- if (node->shared_prefetch_iterator)
- tbm_end_shared_iterate(node->shared_prefetch_iterator);
+ if (node->pf_iterator.exhausted)
+ bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
- node->prefetch_iterator = NULL;
node->initialized = false;
- node->shared_prefetch_iterator = NULL;
node->pvmbuffer = InvalidBuffer;
node->recheck = true;
node->blockno = InvalidBlockNumber;
@@ -624,12 +650,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* release bitmaps and buffers if any
*/
- if (node->prefetch_iterator)
- tbm_end_iterate(node->prefetch_iterator);
+ if (!node->pf_iterator.exhausted)
+ bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->shared_prefetch_iterator)
- tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
}
@@ -667,11 +691,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
- scanstate->prefetch_iterator = NULL;
scanstate->prefetch_pages = 0;
scanstate->prefetch_target = 0;
scanstate->initialized = false;
- scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
scanstate->recheck = true;
scanstate->blockno = InvalidBlockNumber;
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index 92b829cebc..e520186b41 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -20,13 +20,11 @@
#include "storage/buf.h"
#include "storage/spin.h"
#include "utils/relcache.h"
+#include "executor/nodeBitmapHeapscan.h"
struct ParallelTableScanDescData;
-struct TBMIterator;
-struct TBMSharedIterator;
-
/*
* Generic descriptor for table scans. This is the base-class for table scans,
* which needs to be embedded in the scans of individual AMs.
@@ -44,8 +42,7 @@ typedef struct TableScanDescData
ItemPointerData rs_maxtid;
/* Only used for Bitmap table scans */
- struct TBMIterator *tbmiterator;
- struct TBMSharedIterator *shared_tbmiterator;
+ BitmapHeapIterator rs_bhs_iterator;
/*
* Information about type and behaviour of the scan, a bitmask of members
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 42c67a128e..618ab2b449 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -938,13 +938,9 @@ static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
- TableScanDesc result;
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
- result->shared_tbmiterator = NULL;
- result->tbmiterator = NULL;
- return result;
+ return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
}
/*
@@ -991,20 +987,9 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN)
- {
- if (scan->shared_tbmiterator)
- {
- tbm_end_shared_iterate(scan->shared_tbmiterator);
- scan->shared_tbmiterator = NULL;
- }
-
- if (scan->tbmiterator)
- {
- tbm_end_iterate(scan->tbmiterator);
- scan->tbmiterator = NULL;
- }
- }
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&
+ !scan->rs_bhs_iterator.exhausted)
+ bhs_end_iterate(&scan->rs_bhs_iterator);
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1016,20 +1001,9 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN)
- {
- if (scan->shared_tbmiterator)
- {
- tbm_end_shared_iterate(scan->shared_tbmiterator);
- scan->shared_tbmiterator = NULL;
- }
-
- if (scan->tbmiterator)
- {
- tbm_end_iterate(scan->tbmiterator);
- scan->tbmiterator = NULL;
- }
- }
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&
+ !scan->rs_bhs_iterator.exhausted)
+ bhs_end_iterate(&scan->rs_bhs_iterator);
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h
index ea003a9caa..7064f54686 100644
--- a/src/include/executor/nodeBitmapHeapscan.h
+++ b/src/include/executor/nodeBitmapHeapscan.h
@@ -29,4 +29,11 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
ParallelWorkerContext *pwcxt);
+extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);
+extern void bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,
+ dsa_area *dsa, dsa_pointer dsp);
+
+extern void bhs_end_iterate(BitmapHeapIterator *iterator);
+
+
#endif /* NODEBITMAPHEAPSCAN_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index d96703b04d..714eb3e534 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1787,6 +1787,13 @@ typedef struct ParallelBitmapHeapState
ConditionVariable cv;
} ParallelBitmapHeapState;
+typedef struct BitmapHeapIterator
+{
+ struct TBMIterator *serial;
+ struct TBMSharedIterator *parallel;
+ bool exhausted;
+} BitmapHeapIterator;
+
/* ----------------
* BitmapHeapScanState information
*
@@ -1795,12 +1802,11 @@ typedef struct ParallelBitmapHeapState
* 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
- * prefetch_iterator iterator for prefetching ahead of current page
+ * pf_iterator for prefetching ahead of current page
* prefetch_pages # pages prefetch iterator is ahead of current
* prefetch_target current target prefetch distance
* prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
- * shared_prefetch_iterator shared iterator for prefetching
* pstate shared state for parallel bitmap scan
* recheck do current page's tuples need recheck
* blockno used to validate pf and current block in sync
@@ -1815,12 +1821,11 @@ typedef struct BitmapHeapScanState
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
- TBMIterator *prefetch_iterator;
int prefetch_pages;
int prefetch_target;
int prefetch_maximum;
bool initialized;
- TBMSharedIterator *shared_prefetch_iterator;
+ BitmapHeapIterator pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
BlockNumber blockno;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f3b8641d76..2348a8793e 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -259,6 +259,7 @@ BitString
BitmapAnd
BitmapAndPath
BitmapAndState
+BitmapHeapIterator
BitmapHeapPath
BitmapHeapScan
BitmapHeapScanState
--
2.40.1
[text/x-diff] v15-0011-Push-BitmapHeapScan-prefetch-code-into-heapam.c.patch (47.7K, ../20240405080634.d7c23f46xjhhxf5q@liskov/12-v15-0011-Push-BitmapHeapScan-prefetch-code-into-heapam.c.patch)
download | inline diff:
From d141ad447470458de47c6c9b54c97491bb9f168c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 09:42:23 -0400
Subject: [PATCH v15 11/13] Push BitmapHeapScan prefetch code into heapam.c
7566751f0ce9 eliminated a table AM layering violation for the current
block in a BitmapHeapScan but left the violation in BitmapHeapScan
prefetching code.
To resolve this, move the BitmapHeapScan prefetching logic entirely into
heap AM code. There is a fair amount of setup that is specific to
BitmapHeapScan and once the state associated with that lives in the scan
descriptor, it made more sense to make dedicated
begin_scan()/rescan()/endscan() routines for BitmapHeapScan.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 129 ++++++-
src/backend/access/heap/heapam_handler.c | 306 ++++++++++++++--
src/backend/executor/nodeBitmapHeapscan.c | 410 +++-------------------
src/include/access/heapam.h | 62 +++-
src/include/access/relscan.h | 3 -
src/include/access/tableam.h | 74 ++--
src/include/executor/nodeBitmapHeapscan.h | 6 +
src/include/nodes/execnodes.h | 21 --
src/tools/pgindent/typedefs.list | 1 +
9 files changed, 553 insertions(+), 459 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 10f2faaa60..a21ec92d71 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -938,6 +938,48 @@ continue_page:
* ----------------------------------------------------------------
*/
+TableScanDesc
+heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags)
+{
+ BitmapHeapScanDesc scan;
+
+ /*
+ * increment relation ref count while scanning relation
+ *
+ * This is just to make really sure the relcache entry won't go away while
+ * the scan has a pointer to it. Caller should be holding the rel open
+ * anyway, so this is redundant in all normal scenarios...
+ */
+ RelationIncrementReferenceCount(relation);
+ scan = (BitmapHeapScanDesc) palloc(sizeof(BitmapHeapScanDescData));
+
+ scan->heap_common.rs_base.rs_rd = relation;
+ scan->heap_common.rs_base.rs_snapshot = snapshot;
+ scan->heap_common.rs_base.rs_nkeys = 0;
+ scan->heap_common.rs_base.rs_flags = flags;
+ scan->heap_common.rs_base.rs_parallel = NULL;
+ scan->heap_common.rs_strategy = NULL;
+
+ Assert(snapshot && IsMVCCSnapshot(snapshot));
+
+ /* we only need to set this up once */
+ scan->heap_common.rs_ctup.t_tableOid = RelationGetRelid(relation);
+
+ scan->heap_common.rs_parallelworkerdata = NULL;
+ scan->heap_common.rs_base.rs_key = NULL;
+
+ initscan(&scan->heap_common, NULL, false);
+
+ scan->iterator.serial = NULL;
+ scan->iterator.parallel = NULL;
+ scan->vmbuffer = InvalidBuffer;
+ scan->pf_iterator.serial = NULL;
+ scan->pf_iterator.parallel = NULL;
+ scan->pvmbuffer = InvalidBuffer;
+
+ return (TableScanDesc) scan;
+}
+
TableScanDesc
heap_beginscan(Relation relation, Snapshot snapshot,
@@ -967,8 +1009,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
scan->rs_base.rs_flags = flags;
scan->rs_base.rs_parallel = parallel_scan;
scan->rs_strategy = NULL; /* set in initscan */
- scan->rs_vmbuffer = InvalidBuffer;
- scan->rs_empty_tuples_pending = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1026,6 +1066,35 @@ heap_beginscan(Relation relation, Snapshot snapshot,
return (TableScanDesc) scan;
}
+/*
+ * Cleanup BitmapHeapScan table state
+ */
+void
+heap_endscan_bm(TableScanDesc sscan)
+{
+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
+
+ if (BufferIsValid(scan->heap_common.rs_cbuf))
+ ReleaseBuffer(scan->heap_common.rs_cbuf);
+
+ if (BufferIsValid(scan->vmbuffer))
+ ReleaseBuffer(scan->vmbuffer);
+
+ if (BufferIsValid(scan->pvmbuffer))
+ ReleaseBuffer(scan->pvmbuffer);
+
+ bhs_end_iterate(&scan->pf_iterator);
+ bhs_end_iterate(&scan->iterator);
+
+ /*
+ * decrement relation reference count and free scan descriptor storage
+ */
+ RelationDecrementReferenceCount(scan->heap_common.rs_base.rs_rd);
+
+ pfree(scan);
+}
+
+
void
heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
bool allow_strat, bool allow_sync, bool allow_pagemode)
@@ -1057,12 +1126,6 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
- if (BufferIsValid(scan->rs_vmbuffer))
- {
- ReleaseBuffer(scan->rs_vmbuffer);
- scan->rs_vmbuffer = InvalidBuffer;
- }
-
/*
* reinitialize scan descriptor
*/
@@ -1082,12 +1145,6 @@ heap_endscan(TableScanDesc sscan)
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
- if (BufferIsValid(scan->rs_vmbuffer))
- {
- ReleaseBuffer(scan->rs_vmbuffer);
- scan->rs_vmbuffer = InvalidBuffer;
- }
-
/*
* decrement relation reference count and free scan descriptor storage
*/
@@ -1334,6 +1391,50 @@ heap_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction,
return true;
}
+void
+heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
+ ParallelBitmapHeapState *pstate, dsa_area *dsa, int pf_maximum)
+{
+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
+
+ if (BufferIsValid(scan->vmbuffer))
+ ReleaseBuffer(scan->vmbuffer);
+ scan->vmbuffer = InvalidBuffer;
+
+ if (BufferIsValid(scan->pvmbuffer))
+ ReleaseBuffer(scan->pvmbuffer);
+ scan->pvmbuffer = InvalidBuffer;
+
+ bhs_end_iterate(&scan->pf_iterator);
+ bhs_end_iterate(&scan->iterator);
+
+ scan->prefetch_maximum = 0;
+ scan->empty_tuples_pending = 0;
+ scan->pstate = NULL;
+ scan->prefetch_target = -1;
+ scan->prefetch_pages = 0;
+ scan->pfblockno = InvalidBlockNumber;
+
+ bhs_begin_iterate(&scan->iterator,
+ tbm, dsa,
+ pstate ?
+ pstate->tbmiterator :
+ InvalidDsaPointer);
+#ifdef USE_PREFETCH
+ if (pf_maximum > 0)
+ {
+ bhs_begin_iterate(&scan->pf_iterator, tbm, dsa,
+ pstate ?
+ pstate->prefetch_iterator :
+ InvalidDsaPointer);
+ }
+#endif /* USE_PREFETCH */
+
+ scan->pstate = pstate;
+ scan->prefetch_maximum = pf_maximum;
+}
+
+
/*
* heap_fetch - retrieve tuple with given tid
*
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 5e57d19d91..c513cbd2da 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -60,6 +60,9 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
OffsetNumber tupoffset);
static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan);
+static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan);
+static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanDesc scan);
+static inline void BitmapPrefetch(BitmapHeapScanDesc scan);
static const TableAmRoutine heapam_methods;
@@ -2186,12 +2189,80 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
* ------------------------------------------------------------------------
*/
+/*
+ * BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
+ *
+ * We keep track of how far the prefetch iterator is ahead of the main
+ * iterator in prefetch_pages. For each block the main iterator returns, we
+ * decrement prefetch_pages.
+ */
+static inline void
+BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ BitmapHeapIterator *prefetch_iterator = &scan->pf_iterator;
+ ParallelBitmapHeapState *pstate = scan->pstate;
+ TBMIterateResult *tbmpre;
+
+ if (pstate == NULL)
+ {
+ if (scan->prefetch_pages > 0)
+ {
+ /* The main iterator has closed the distance by one page */
+ scan->prefetch_pages--;
+ }
+ else if (!prefetch_iterator->exhausted)
+ {
+ /* Do not let the prefetch iterator get behind the main one */
+ tbmpre = bhs_iterate(prefetch_iterator);
+ scan->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
+ }
+ return;
+ }
+
+ /*
+ * Adjusting the prefetch iterator before invoking
+ * table_scan_bitmap_next_block() keeps prefetch distance higher across
+ * the parallel workers.
+ */
+ if (scan->prefetch_maximum > 0)
+ {
+ SpinLockAcquire(&pstate->mutex);
+ if (pstate->prefetch_pages > 0)
+ {
+ pstate->prefetch_pages--;
+ SpinLockRelease(&pstate->mutex);
+ }
+ else
+ {
+ /* Release the mutex before iterating */
+ SpinLockRelease(&pstate->mutex);
+
+ /*
+ * In case of shared mode, we can not ensure that the current
+ * blockno of the main iterator and that of the prefetch iterator
+ * are same. It's possible that whatever blockno we are
+ * prefetching will be processed by another process. Therefore,
+ * we don't validate the blockno here as we do in non-parallel
+ * case.
+ */
+ if (!prefetch_iterator->exhausted)
+ {
+ tbmpre = bhs_iterate(prefetch_iterator);
+ scan->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
+ }
+ }
+ }
+#endif /* USE_PREFETCH */
+}
+
static bool
-heapam_scan_bitmap_next_block(TableScanDesc scan,
- BlockNumber *blockno, bool *recheck,
+heapam_scan_bitmap_next_block(TableScanDesc sscan,
+ bool *recheck,
long *lossy_pages, long *exact_pages)
{
- HeapScanDesc hscan = (HeapScanDesc) scan;
+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
+ HeapScanDesc hscan = &scan->heap_common;
BlockNumber block;
Buffer buffer;
Snapshot snapshot;
@@ -2201,19 +2272,21 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
- *blockno = InvalidBlockNumber;
*recheck = true;
+ BitmapAdjustPrefetchIterator(scan);
+
do
{
CHECK_FOR_INTERRUPTS();
- tbmres = bhs_iterate(&scan->rs_bhs_iterator);
+ Assert(!scan->iterator.exhausted);
+ tbmres = bhs_iterate(&scan->iterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
- Assert(hscan->rs_empty_tuples_pending == 0);
+ Assert(scan->empty_tuples_pending == 0);
return false;
}
@@ -2228,7 +2301,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
} while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks);
/* Got a valid block */
- *blockno = tbmres->blockno;
*recheck = tbmres->recheck;
/*
@@ -2236,15 +2308,15 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* heap, the bitmap entries don't need rechecking, and all tuples on the
* page are visible to our transaction.
*/
- if (!(scan->rs_flags & SO_NEED_TUPLE) &&
+ if (!(hscan->rs_base.rs_flags & SO_NEED_TUPLE) &&
!tbmres->recheck &&
- VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &hscan->rs_vmbuffer))
+ VM_ALL_VISIBLE(hscan->rs_base.rs_rd, tbmres->blockno, &scan->vmbuffer))
{
/* can't be lossy in the skip_fetch case */
Assert(tbmres->ntuples >= 0);
- Assert(hscan->rs_empty_tuples_pending >= 0);
+ Assert(scan->empty_tuples_pending >= 0);
- hscan->rs_empty_tuples_pending += tbmres->ntuples;
+ scan->empty_tuples_pending += tbmres->ntuples;
return true;
}
@@ -2255,18 +2327,18 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* Acquire pin on the target heap page, trading in any pin we held before.
*/
hscan->rs_cbuf = ReleaseAndReadBuffer(hscan->rs_cbuf,
- scan->rs_rd,
+ hscan->rs_base.rs_rd,
block);
hscan->rs_cblock = block;
buffer = hscan->rs_cbuf;
- snapshot = scan->rs_snapshot;
+ snapshot = hscan->rs_base.rs_snapshot;
ntup = 0;
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
- heap_page_prune_opt(scan->rs_rd, buffer);
+ heap_page_prune_opt(hscan->rs_base.rs_rd, buffer);
/*
* We must hold share lock on the buffer content while examining tuple
@@ -2294,7 +2366,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
HeapTupleData heapTuple;
ItemPointerSet(&tid, block, offnum);
- if (heap_hot_search_buffer(&tid, scan->rs_rd, buffer, snapshot,
+ if (heap_hot_search_buffer(&tid, hscan->rs_base.rs_rd, buffer, snapshot,
&heapTuple, NULL, true))
hscan->rs_vistuples[ntup++] = ItemPointerGetOffsetNumber(&tid);
}
@@ -2320,16 +2392,16 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
continue;
loctup.t_data = (HeapTupleHeader) PageGetItem(page, lp);
loctup.t_len = ItemIdGetLength(lp);
- loctup.t_tableOid = scan->rs_rd->rd_id;
+ loctup.t_tableOid = hscan->rs_base.rs_rd->rd_id;
ItemPointerSet(&loctup.t_self, block, offnum);
valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer);
if (valid)
{
hscan->rs_vistuples[ntup++] = offnum;
- PredicateLockTID(scan->rs_rd, &loctup.t_self, snapshot,
+ PredicateLockTID(hscan->rs_base.rs_rd, &loctup.t_self, snapshot,
HeapTupleHeaderGetXmin(loctup.t_data));
}
- HeapCheckForSerializableConflictOut(valid, scan->rs_rd, &loctup,
+ HeapCheckForSerializableConflictOut(valid, hscan->rs_base.rs_rd, &loctup,
buffer, snapshot);
}
}
@@ -2344,6 +2416,18 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
else
(*exact_pages)++;
+ /*
+ * If serial, we can error out if the the prefetch block doesn't stay
+ * ahead of the current block.
+ */
+ if (scan->pstate == NULL &&
+ !scan->pf_iterator.exhausted &&
+ scan->pfblockno < block)
+ elog(ERROR, "prefetch and main iterators are out of sync");
+
+ /* Adjust the prefetch target */
+ BitmapAdjustPrefetchTarget(scan);
+
/*
* Return true to indicate that a valid block was found and the bitmap is
* not exhausted. If there are no visible tuples on this page,
@@ -2354,22 +2438,168 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
return true;
}
+/*
+ * BitmapAdjustPrefetchTarget - Adjust the prefetch target
+ *
+ * Increase prefetch target if it's not yet at the max. Note that
+ * we will increase it to zero after fetching the very first
+ * page/tuple, then to one after the second tuple is fetched, then
+ * it doubles as later pages are fetched.
+ */
+static inline void
+BitmapAdjustPrefetchTarget(BitmapHeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ ParallelBitmapHeapState *pstate = scan->pstate;
+ int prefetch_maximum = scan->prefetch_maximum;
+
+ if (pstate == NULL)
+ {
+ if (scan->prefetch_target >= prefetch_maximum)
+ /* don't increase any further */ ;
+ else if (scan->prefetch_target >= prefetch_maximum / 2)
+ scan->prefetch_target = prefetch_maximum;
+ else if (scan->prefetch_target > 0)
+ scan->prefetch_target *= 2;
+ else
+ scan->prefetch_target++;
+ return;
+ }
+
+ /* Do an unlocked check first to save spinlock acquisitions. */
+ if (pstate->prefetch_target < prefetch_maximum)
+ {
+ SpinLockAcquire(&pstate->mutex);
+ if (pstate->prefetch_target >= prefetch_maximum)
+ /* don't increase any further */ ;
+ else if (pstate->prefetch_target >= prefetch_maximum / 2)
+ pstate->prefetch_target = prefetch_maximum;
+ else if (pstate->prefetch_target > 0)
+ pstate->prefetch_target *= 2;
+ else
+ pstate->prefetch_target++;
+ SpinLockRelease(&pstate->mutex);
+ }
+#endif /* USE_PREFETCH */
+}
+
+
+/*
+ * BitmapPrefetch - Prefetch, if prefetch_pages are behind prefetch_target
+ */
+static inline void
+BitmapPrefetch(BitmapHeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ ParallelBitmapHeapState *pstate = scan->pstate;
+ BitmapHeapIterator *prefetch_iterator = &scan->pf_iterator;
+ Relation rel = scan->heap_common.rs_base.rs_rd;
+
+ if (pstate == NULL)
+ {
+ if (!prefetch_iterator->exhausted)
+ {
+ while (scan->prefetch_pages < scan->prefetch_target)
+ {
+ TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
+ bool skip_fetch;
+
+ if (tbmpre == NULL)
+ {
+ /* No more pages to prefetch */
+ bhs_end_iterate(prefetch_iterator);
+ break;
+ }
+ scan->prefetch_pages++;
+ scan->pfblockno = tbmpre->blockno;
+
+ /*
+ * If we expect not to have to actually read this heap page,
+ * skip this prefetch call, but continue to run the prefetch
+ * logic normally. (Would it be better not to increment
+ * prefetch_pages?)
+ */
+ skip_fetch = (!(scan->heap_common.rs_base.rs_flags & SO_NEED_TUPLE) &&
+ !tbmpre->recheck &&
+ VM_ALL_VISIBLE(rel,
+ tbmpre->blockno,
+ &scan->pvmbuffer));
+
+ if (!skip_fetch)
+ PrefetchBuffer(rel, MAIN_FORKNUM, tbmpre->blockno);
+ }
+ }
+
+ return;
+ }
+
+ if (pstate->prefetch_pages < pstate->prefetch_target)
+ {
+ if (!prefetch_iterator->exhausted)
+ {
+ while (1)
+ {
+ TBMIterateResult *tbmpre;
+ bool do_prefetch = false;
+ bool skip_fetch;
+
+ /*
+ * Recheck under the mutex. If some other process has already
+ * done enough prefetching then we need not to do anything.
+ */
+ SpinLockAcquire(&pstate->mutex);
+ if (pstate->prefetch_pages < pstate->prefetch_target)
+ {
+ pstate->prefetch_pages++;
+ do_prefetch = true;
+ }
+ SpinLockRelease(&pstate->mutex);
+
+ if (!do_prefetch)
+ return;
+
+ tbmpre = bhs_iterate(prefetch_iterator);
+ if (tbmpre == NULL)
+ {
+ /* No more pages to prefetch */
+ bhs_end_iterate(prefetch_iterator);
+ break;
+ }
+
+ scan->pfblockno = tbmpre->blockno;
+
+ /* As above, skip prefetch if we expect not to need page */
+ skip_fetch = (!(scan->heap_common.rs_base.rs_flags & SO_NEED_TUPLE) &&
+ !tbmpre->recheck &&
+ VM_ALL_VISIBLE(rel,
+ tbmpre->blockno,
+ &scan->pvmbuffer));
+
+ if (!skip_fetch)
+ PrefetchBuffer(rel, MAIN_FORKNUM, tbmpre->blockno);
+ }
+ }
+ }
+#endif /* USE_PREFETCH */
+}
+
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
TupleTableSlot *slot)
{
- HeapScanDesc hscan = (HeapScanDesc) scan;
+ BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
+ HeapScanDesc hscan = &bscan->heap_common;
OffsetNumber targoffset;
Page page;
ItemId lp;
- if (hscan->rs_empty_tuples_pending > 0)
+ if (bscan->empty_tuples_pending > 0)
{
/*
* If we don't have to fetch the tuple, just return nulls.
*/
ExecStoreAllNullTuple(slot);
- hscan->rs_empty_tuples_pending--;
+ bscan->empty_tuples_pending--;
return true;
}
@@ -2379,6 +2609,36 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
return false;
+#ifdef USE_PREFETCH
+
+ /*
+ * Try to prefetch at least a few pages even before we get to the second
+ * page if we don't stop reading after the first tuple.
+ */
+ if (!bscan->pstate)
+ {
+ if (bscan->prefetch_target < bscan->prefetch_maximum)
+ bscan->prefetch_target++;
+ }
+ else if (bscan->pstate->prefetch_target < bscan->prefetch_maximum)
+ {
+ /* take spinlock while updating shared state */
+ SpinLockAcquire(&bscan->pstate->mutex);
+ if (bscan->pstate->prefetch_target < bscan->prefetch_maximum)
+ bscan->pstate->prefetch_target++;
+ SpinLockRelease(&bscan->pstate->mutex);
+ }
+
+ /*
+ * We issue prefetch requests *after* fetching the current page to try to
+ * avoid having prefetching interfere with the main I/O. Also, this should
+ * happen only when we have determined there is still something to do on
+ * the current page, else we may uselessly prefetch the same page we are
+ * just about to request for real.
+ */
+ BitmapPrefetch(bscan);
+#endif /* USE_PREFETCH */
+
targoffset = hscan->rs_vistuples[hscan->rs_cindex];
page = BufferGetPage(hscan->rs_cbuf);
lp = PageGetItemId(page, targoffset);
@@ -2704,6 +2964,10 @@ static const TableAmRoutine heapam_methods = {
.scan_set_tidrange = heap_set_tidrange,
.scan_getnextslot_tidrange = heap_getnextslot_tidrange,
+ .scan_rescan_bm = heap_rescan_bm,
+ .scan_begin_bm = heap_beginscan_bm,
+ .scan_end_bm = heap_endscan_bm,
+
.parallelscan_estimate = table_block_parallelscan_estimate,
.parallelscan_initialize = table_block_parallelscan_initialize,
.parallelscan_reinitialize = table_block_parallelscan_reinitialize,
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 70b560b8ee..d22a433f06 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -51,10 +51,6 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
-static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);
-static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
-static inline void BitmapPrefetch(BitmapHeapScanState *node,
- TableScanDesc scan);
static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
/*
@@ -122,20 +118,9 @@ bhs_end_iterate(BitmapHeapIterator *iterator)
static TupleTableSlot *
BitmapHeapNext(BitmapHeapScanState *node)
{
- ExprContext *econtext;
- TableScanDesc scan;
- TIDBitmap *tbm;
- TupleTableSlot *slot;
- ParallelBitmapHeapState *pstate = node->pstate;
- dsa_area *dsa = node->ss.ps.state->es_query_dsa;
-
- /*
- * extract necessary information from index scan node
- */
- econtext = node->ss.ps.ps_ExprContext;
- slot = node->ss.ss_ScanTupleSlot;
- scan = node->ss.ss_currentScanDesc;
- tbm = node->tbm;
+ ExprContext *econtext = node->ss.ps.ps_ExprContext;
+ TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
+ TableScanDesc scan = node->ss.ss_currentScanDesc;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
@@ -146,25 +131,37 @@ BitmapHeapNext(BitmapHeapScanState *node)
* prefetching. node->prefetch_pages tracks exactly how many pages ahead
* the prefetch iterator is. Also, node->prefetch_target tracks the
* desired prefetch distance, which starts small and increases up to the
- * node->prefetch_maximum. This is to avoid doing a lot of prefetching in
+ * scan->prefetch_maximum. This is to avoid doing a lot of prefetching in
* a scan that stops after a few tuples because of a LIMIT.
*/
if (!node->initialized)
{
+ Relation rel = node->ss.ss_currentRelation;
+ bool pf_maximum = 0;
+ bool init_shared_state = false;
+ uint32 extra_flags = 0;
+
/*
* The leader will immediately come out of the function, but others
* will be blocked until leader populates the TBM and wakes them up.
*/
- bool init_shared_state = node->pstate ?
+ init_shared_state = node->pstate ?
BitmapShouldInitializeSharedState(node->pstate) : false;
- if (!pstate || init_shared_state)
+ /*
+ * Maximum number of prefetches for the tablespace if configured,
+ * otherwise the current value of the effective_io_concurrency GUC.
+ */
+#ifdef USE_PREFETCH
+ pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);
+#endif
+
+ if (!node->pstate || init_shared_state)
{
- tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
+ node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
- if (!tbm || !IsA(tbm, TIDBitmap))
+ if (!node->tbm || !IsA(node->tbm, TIDBitmap))
elog(ERROR, "unrecognized result from subplan");
- node->tbm = tbm;
if (init_shared_state)
{
@@ -173,113 +170,52 @@ BitmapHeapNext(BitmapHeapScanState *node)
* dsa_pointer of the iterator state which will be used by
* multiple processes to iterate jointly.
*/
- pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
+ node->pstate->tbmiterator = tbm_prepare_shared_iterate(node->tbm);
+
#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
+ if (pf_maximum > 0)
{
- pstate->prefetch_iterator =
- tbm_prepare_shared_iterate(tbm);
-
- /*
- * We don't need the mutex here as we haven't yet woke up
- * others.
- */
- pstate->prefetch_pages = 0;
- pstate->prefetch_target = -1;
+ node->pstate->prefetch_iterator =
+ tbm_prepare_shared_iterate(node->tbm);
}
#endif
/* We have initialized the shared state so wake up others. */
- BitmapDoneInitializingSharedState(pstate);
+ BitmapDoneInitializingSharedState(node->pstate);
}
}
/*
- * If this is the first scan of the underlying table, create the table
- * scan descriptor and begin the scan.
+ * We can potentially skip fetching heap pages if we do not need any
+ * columns of the table, either for checking non-indexable quals or
+ * for returning data. This test is a bit simplistic, as it checks
+ * the stronger condition that there's no qual or return tlist at all.
+ * But in most cases it's probably not worth working harder than that.
*/
- if (!scan)
- {
- uint32 extra_flags = 0;
-
- /*
- * We can potentially skip fetching heap pages if we do not need
- * any columns of the table, either for checking non-indexable
- * quals or for returning data. This test is a bit simplistic, as
- * it checks the stronger condition that there's no qual or return
- * tlist at all. But in most cases it's probably not worth working
- * harder than that.
- */
- if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)
- extra_flags |= SO_NEED_TUPLE;
-
- scan = table_beginscan_bm(node->ss.ss_currentRelation,
- node->ss.ps.state->es_snapshot,
- 0,
- NULL,
- extra_flags);
-
- node->ss.ss_currentScanDesc = scan;
- }
-
- bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa,
- pstate ?
- pstate->tbmiterator :
- InvalidDsaPointer);
-
-#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
- {
- bhs_begin_iterate(&node->pf_iterator, tbm, dsa,
- pstate ?
- pstate->prefetch_iterator :
- InvalidDsaPointer);
- /* Only used for serial BHS */
- node->prefetch_pages = 0;
- node->prefetch_target = -1;
- }
-#endif /* USE_PREFETCH */
-
+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)
+ extra_flags |= SO_NEED_TUPLE;
+
+ scan = table_beginscan_bm(node->ss.ss_currentScanDesc,
+ rel,
+ node->ss.ps.state->es_snapshot,
+ extra_flags,
+ pf_maximum,
+ node->tbm,
+ node->pstate,
+ node->ss.ps.state->es_query_dsa);
+
+ node->ss.ss_currentScanDesc = scan;
node->initialized = true;
goto new_page;
}
+
for (;;)
{
while (table_scan_bitmap_next_tuple(scan, slot))
{
CHECK_FOR_INTERRUPTS();
-#ifdef USE_PREFETCH
-
- /*
- * Try to prefetch at least a few pages even before we get to the
- * second page if we don't stop reading after the first tuple.
- */
- if (!pstate)
- {
- if (node->prefetch_target < node->prefetch_maximum)
- node->prefetch_target++;
- }
- else if (pstate->prefetch_target < node->prefetch_maximum)
- {
- /* take spinlock while updating shared state */
- SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_target < node->prefetch_maximum)
- pstate->prefetch_target++;
- SpinLockRelease(&pstate->mutex);
- }
-#endif /* USE_PREFETCH */
-
- /*
- * We issue prefetch requests *after* fetching the current page to
- * try to avoid having prefetching interfere with the main I/O.
- * Also, this should happen only when we have determined there is
- * still something to do on the current page, else we may
- * uselessly prefetch the same page we are just about to request
- * for real.
- */
- BitmapPrefetch(node, scan);
/*
* If we are using lossy info, we have to recheck the qual
@@ -303,23 +239,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
new_page:
- BitmapAdjustPrefetchIterator(node);
-
- if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck,
+ if (!table_scan_bitmap_next_block(scan, &node->recheck,
&node->lossy_pages, &node->exact_pages))
break;
-
- /*
- * If serial, we can error out if the the prefetch block doesn't stay
- * ahead of the current block.
- */
- if (node->pstate == NULL &&
- !node->pf_iterator.exhausted &&
- node->pfblockno < node->blockno)
- elog(ERROR, "prefetch and main iterators are out of sync");
-
- /* Adjust the prefetch target */
- BitmapAdjustPrefetchTarget(node);
}
/*
@@ -343,215 +265,6 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
ConditionVariableBroadcast(&pstate->cv);
}
-/*
- * BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
- *
- * We keep track of how far the prefetch iterator is ahead of the main
- * iterator in prefetch_pages. For each block the main iterator returns, we
- * decrement prefetch_pages.
- */
-static inline void
-BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
-{
-#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;
- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
- TBMIterateResult *tbmpre;
-
- if (pstate == NULL)
- {
- if (node->prefetch_pages > 0)
- {
- /* The main iterator has closed the distance by one page */
- node->prefetch_pages--;
- }
- else if (!prefetch_iterator->exhausted)
- {
- /* Do not let the prefetch iterator get behind the main one */
- tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
- }
- return;
- }
-
- /*
- * Adjusting the prefetch iterator before invoking
- * table_scan_bitmap_next_block() keeps prefetch distance higher across
- * the parallel workers.
- */
- if (node->prefetch_maximum > 0)
- {
- SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_pages > 0)
- {
- pstate->prefetch_pages--;
- SpinLockRelease(&pstate->mutex);
- }
- else
- {
- /* Release the mutex before iterating */
- SpinLockRelease(&pstate->mutex);
-
- /*
- * In case of shared mode, we can not ensure that the current
- * blockno of the main iterator and that of the prefetch iterator
- * are same. It's possible that whatever blockno we are
- * prefetching will be processed by another process. Therefore,
- * we don't validate the blockno here as we do in non-parallel
- * case.
- */
- if (!prefetch_iterator->exhausted)
- {
- tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
- }
- }
- }
-#endif /* USE_PREFETCH */
-}
-
-/*
- * BitmapAdjustPrefetchTarget - Adjust the prefetch target
- *
- * Increase prefetch target if it's not yet at the max. Note that
- * we will increase it to zero after fetching the very first
- * page/tuple, then to one after the second tuple is fetched, then
- * it doubles as later pages are fetched.
- */
-static inline void
-BitmapAdjustPrefetchTarget(BitmapHeapScanState *node)
-{
-#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;
-
- if (pstate == NULL)
- {
- if (node->prefetch_target >= node->prefetch_maximum)
- /* don't increase any further */ ;
- else if (node->prefetch_target >= node->prefetch_maximum / 2)
- node->prefetch_target = node->prefetch_maximum;
- else if (node->prefetch_target > 0)
- node->prefetch_target *= 2;
- else
- node->prefetch_target++;
- return;
- }
-
- /* Do an unlocked check first to save spinlock acquisitions. */
- if (pstate->prefetch_target < node->prefetch_maximum)
- {
- SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_target >= node->prefetch_maximum)
- /* don't increase any further */ ;
- else if (pstate->prefetch_target >= node->prefetch_maximum / 2)
- pstate->prefetch_target = node->prefetch_maximum;
- else if (pstate->prefetch_target > 0)
- pstate->prefetch_target *= 2;
- else
- pstate->prefetch_target++;
- SpinLockRelease(&pstate->mutex);
- }
-#endif /* USE_PREFETCH */
-}
-
-/*
- * BitmapPrefetch - Prefetch, if prefetch_pages are behind prefetch_target
- */
-static inline void
-BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
-{
-#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;
- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
-
- if (pstate == NULL)
- {
- if (!prefetch_iterator->exhausted)
- {
- while (node->prefetch_pages < node->prefetch_target)
- {
- TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
- bool skip_fetch;
-
- if (tbmpre == NULL)
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- break;
- }
- node->prefetch_pages++;
- node->pfblockno = tbmpre->blockno;
-
- /*
- * If we expect not to have to actually read this heap page,
- * skip this prefetch call, but continue to run the prefetch
- * logic normally. (Would it be better not to increment
- * prefetch_pages?)
- */
- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- !tbmpre->recheck &&
- VM_ALL_VISIBLE(node->ss.ss_currentRelation,
- tbmpre->blockno,
- &node->pvmbuffer));
-
- if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);
- }
- }
-
- return;
- }
-
- if (pstate->prefetch_pages < pstate->prefetch_target)
- {
- if (!prefetch_iterator->exhausted)
- {
- while (1)
- {
- TBMIterateResult *tbmpre;
- bool do_prefetch = false;
- bool skip_fetch;
-
- /*
- * Recheck under the mutex. If some other process has already
- * done enough prefetching then we need not to do anything.
- */
- SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_pages < pstate->prefetch_target)
- {
- pstate->prefetch_pages++;
- do_prefetch = true;
- }
- SpinLockRelease(&pstate->mutex);
-
- if (!do_prefetch)
- return;
-
- tbmpre = bhs_iterate(prefetch_iterator);
- if (tbmpre == NULL)
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- break;
- }
-
- node->pfblockno = tbmpre->blockno;
-
- /* As above, skip prefetch if we expect not to need page */
- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- !tbmpre->recheck &&
- VM_ALL_VISIBLE(node->ss.ss_currentRelation,
- tbmpre->blockno,
- &node->pvmbuffer));
-
- if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);
- }
- }
- }
-#endif /* USE_PREFETCH */
-}
-
/*
* BitmapHeapRecheck -- access method routine to recheck a tuple in EvalPlanQual
*/
@@ -597,19 +310,12 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
if (node->ss.ss_currentScanDesc)
table_rescan(node->ss.ss_currentScanDesc, NULL);
- /* release bitmaps and buffers if any */
- if (node->pf_iterator.exhausted)
- bhs_end_iterate(&node->pf_iterator);
+ /* release bitmaps if any */
if (node->tbm)
tbm_free(node->tbm);
- if (node->pvmbuffer != InvalidBuffer)
- ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
node->initialized = false;
- node->pvmbuffer = InvalidBuffer;
node->recheck = true;
- node->blockno = InvalidBlockNumber;
- node->pfblockno = InvalidBlockNumber;
ExecScanReScan(&node->ss);
@@ -648,14 +354,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
table_endscan(scanDesc);
/*
- * release bitmaps and buffers if any
+ * release bitmaps if any
*/
- if (!node->pf_iterator.exhausted)
- bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->pvmbuffer != InvalidBuffer)
- ReleaseBuffer(node->pvmbuffer);
}
/* ----------------------------------------------------------------
@@ -688,16 +390,11 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ps.ExecProcNode = ExecBitmapHeapScan;
scanstate->tbm = NULL;
- scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
- scanstate->prefetch_pages = 0;
- scanstate->prefetch_target = 0;
scanstate->initialized = false;
scanstate->pstate = NULL;
scanstate->recheck = true;
- scanstate->blockno = InvalidBlockNumber;
- scanstate->pfblockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
@@ -737,13 +434,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->bitmapqualorig =
ExecInitQual(node->bitmapqualorig, (PlanState *) scanstate);
- /*
- * Maximum number of prefetches for the tablespace if configured,
- * otherwise the current value of the effective_io_concurrency GUC.
- */
- scanstate->prefetch_maximum =
- get_tablespace_io_concurrency(currentRelation->rd_rel->reltablespace);
-
scanstate->ss.ss_currentRelation = currentRelation;
/*
@@ -827,7 +517,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
/* Initialize the mutex */
SpinLockInit(&pstate->mutex);
pstate->prefetch_pages = 0;
- pstate->prefetch_target = 0;
+ pstate->prefetch_target = -1;
pstate->state = BM_INITIAL;
ConditionVariableInit(&pstate->cv);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 750ea30852..73690d15c5 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -76,22 +76,60 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /* these fields only used in page-at-a-time mode and for bitmap scans */
+ int rs_cindex; /* current tuple's index in vistuples */
+ int rs_ntuples; /* number of visible tuples on page */
+ OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */
+
+} HeapScanDescData;
+typedef struct HeapScanDescData *HeapScanDesc;
+
+typedef struct BitmapHeapScanDescData
+{
+ /* All the non-BitmapHeapScan specific members */
+ HeapScanDescData heap_common;
+
+ /*
+ * Members common to Parallel and Serial BitmapHeapScan
+ */
+ struct BitmapHeapIterator iterator;
+ struct BitmapHeapIterator pf_iterator;
+
+ /* maximum value for prefetch_target */
+ int prefetch_maximum;
+
/*
* These fields are only used for bitmap scans for the "skip fetch"
* optimization. Bitmap scans needing no fields from the heap may skip
* fetching an all visible block, instead using the number of tuples per
* block reported by the bitmap to determine how many NULL-filled tuples
- * to return.
+ * to return. They are common to parallel and serial BitmapHeapScans
*/
- Buffer rs_vmbuffer;
- int rs_empty_tuples_pending;
+ Buffer vmbuffer;
+ /* buffer for visibility-map lookups of prefetched pages */
+ Buffer pvmbuffer;
+ int empty_tuples_pending;
+
+ /*
+ * Parallel-only members
+ */
+
+ struct ParallelBitmapHeapState *pstate;
+
+ /*
+ * Serial-only members
+ */
+
+ /* Current target for prefetch distance */
+ int prefetch_target;
+ /* # pages prefetch iterator is ahead of current */
+ int prefetch_pages;
+ /* used to validate prefetch block stays ahead of current block */
+ BlockNumber pfblockno;
+} BitmapHeapScanDescData;
+
+typedef struct BitmapHeapScanDescData *BitmapHeapScanDesc;
- /* these fields only used in page-at-a-time mode and for bitmap scans */
- int rs_cindex; /* current tuple's index in vistuples */
- int rs_ntuples; /* number of visible tuples on page */
- OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */
-} HeapScanDescData;
-typedef struct HeapScanDescData *HeapScanDesc;
/*
* Descriptor for fetches from heap via an index.
@@ -289,6 +327,12 @@ extern void heap_set_tidrange(TableScanDesc sscan, ItemPointer mintid,
extern bool heap_getnextslot_tidrange(TableScanDesc sscan,
ScanDirection direction,
TupleTableSlot *slot);
+extern TableScanDesc heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags);
+
+extern void heap_endscan_bm(TableScanDesc scan);
+extern void heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
+ ParallelBitmapHeapState *pstate, dsa_area *dsa, int pf_maximum);
+
extern bool heap_fetch(Relation relation, Snapshot snapshot,
HeapTuple tuple, Buffer *userbuf, bool keep_buf);
extern bool heap_hot_search_buffer(ItemPointer tid, Relation relation,
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index e520186b41..855b9558bf 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -41,9 +41,6 @@ typedef struct TableScanDescData
ItemPointerData rs_mintid;
ItemPointerData rs_maxtid;
- /* Only used for Bitmap table scans */
- BitmapHeapIterator rs_bhs_iterator;
-
/*
* Information about type and behaviour of the scan, a bitmask of members
* of the ScanOptions enum (see tableam.h).
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 618ab2b449..138bf5f6ed 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -391,6 +391,23 @@ typedef struct TableAmRoutine
ScanDirection direction,
TupleTableSlot *slot);
+ /*
+ * TODO: add comment
+ */
+ TableScanDesc (*scan_begin_bm) (Relation rel,
+ Snapshot snapshot,
+ uint32 flags);
+
+ void (*scan_rescan_bm) (TableScanDesc scan, TIDBitmap *tbm,
+ ParallelBitmapHeapState *pstate, dsa_area *dsa,
+ int pf_maximum);
+
+ /*
+ * Release resources and deallocate scan. If TableScanDesc.temp_snap,
+ * TableScanDesc.rs_snapshot needs to be unregistered.
+ */
+ void (*scan_end_bm) (TableScanDesc scan);
+
/* ------------------------------------------------------------------------
* Parallel table scan related functions.
* ------------------------------------------------------------------------
@@ -774,9 +791,9 @@ typedef struct TableAmRoutine
*/
/*
- * Prepare to fetch / check / return tuples from `blockno` as part of a
- * bitmap table scan. `scan` was started via table_beginscan_bm(). Return
- * false if the bitmap is exhausted and true otherwise.
+ * Prepare to fetch / check / return tuples as part of a bitmap table
+ * scan. `scan` was started via table_beginscan_bm(). Return false if the
+ * bitmap is exhausted and true otherwise.
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
@@ -785,22 +802,11 @@ typedef struct TableAmRoutine
* lossy_pages is incremented if the bitmap is lossy for the selected
* block; otherwise, exact_pages is incremented.
*
- * XXX: Currently this may only be implemented if the AM uses md.c as its
- * storage manager, and uses ItemPointer->ip_blkid in a manner that maps
- * blockids directly to the underlying storage. nodeBitmapHeapscan.c
- * performs prefetching directly using that interface. This probably
- * needs to be rectified at a later point.
- *
- * XXX: Currently this may only be implemented if the AM uses the
- * visibilitymap, as nodeBitmapHeapscan.c unconditionally accesses it to
- * perform prefetching. This probably needs to be rectified at a later
- * point.
- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- BlockNumber *blockno, bool *recheck,
+ bool *recheck,
long *lossy_pages, long *exact_pages);
/*
@@ -929,18 +935,26 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
}
/*
- * table_beginscan_bm is an alternative entry point for setting up a
- * TableScanDesc for a bitmap heap scan. Although that scan technology is
- * really quite unlike a standard seqscan, there is just enough commonality to
- * make it worth using the same data structure.
+ * table_beginscan_bm is the entry point for setting up a TableScanDesc for a
+ * bitmap heap scan.
*/
static inline TableScanDesc
-table_beginscan_bm(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key, uint32 extra_flags)
+table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,
+ uint32 extra_flags, int pf_maximum, TIDBitmap *tbm,
+ ParallelBitmapHeapState *pstate, dsa_area *dsa)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
+ /*
+ * If this is the first scan of the underlying table, create the table
+ * scan descriptor and begin the scan.
+ */
+ if (!scan)
+ scan = rel->rd_tableam->scan_begin_bm(rel, snapshot, flags);
+
+ scan->rs_rd->rd_tableam->scan_rescan_bm(scan, tbm, pstate, dsa, pf_maximum);
+
+ return scan;
}
/*
@@ -987,9 +1001,11 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&
- !scan->rs_bhs_iterator.exhausted)
- bhs_end_iterate(&scan->rs_bhs_iterator);
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN)
+ {
+ scan->rs_rd->rd_tableam->scan_end_bm(scan);
+ return;
+ }
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1001,10 +1017,6 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&
- !scan->rs_bhs_iterator.exhausted)
- bhs_end_iterate(&scan->rs_bhs_iterator);
-
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
@@ -1979,7 +1991,7 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- BlockNumber *blockno, bool *recheck,
+ bool *recheck,
long *lossy_pages,
long *exact_pages)
{
@@ -1992,7 +2004,7 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- blockno, recheck,
+ recheck,
lossy_pages, exact_pages);
}
diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h
index 7064f54686..f3c92f7a8a 100644
--- 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);
+typedef struct BitmapHeapIterator
+{
+ struct TBMIterator *serial;
+ struct TBMSharedIterator *parallel;
+ bool exhausted;
+} BitmapHeapIterator;
extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);
extern void bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 714eb3e534..49ce4ff9a7 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1787,30 +1787,16 @@ typedef struct ParallelBitmapHeapState
ConditionVariable cv;
} ParallelBitmapHeapState;
-typedef struct BitmapHeapIterator
-{
- struct TBMIterator *serial;
- struct TBMSharedIterator *parallel;
- bool exhausted;
-} BitmapHeapIterator;
-
/* ----------------
* BitmapHeapScanState information
*
* bitmapqualorig execution state for bitmapqualorig expressions
* tbm bitmap obtained from child index scan(s)
- * 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
- * pf_iterator for prefetching ahead of current page
- * prefetch_pages # pages prefetch iterator is ahead of current
- * prefetch_target current target prefetch distance
- * prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
* pstate shared state for parallel bitmap scan
* recheck do current page's tuples need recheck
- * blockno used to validate pf and current block in sync
- * pfblockno used to validate pf stays ahead of current block
* ----------------
*/
typedef struct BitmapHeapScanState
@@ -1818,18 +1804,11 @@ typedef struct BitmapHeapScanState
ScanState ss; /* its first field is NodeTag */
ExprState *bitmapqualorig;
TIDBitmap *tbm;
- Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
- int prefetch_pages;
- int prefetch_target;
- int prefetch_maximum;
bool initialized;
- BitmapHeapIterator pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
- BlockNumber blockno;
- BlockNumber pfblockno;
} BitmapHeapScanState;
/* ----------------
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2348a8793e..443b01c053 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -263,6 +263,7 @@ BitmapHeapIterator
BitmapHeapPath
BitmapHeapScan
BitmapHeapScanState
+BitmapHeapScanDesc
BitmapIndexScan
BitmapIndexScanState
BitmapOr
--
2.40.1
[text/x-diff] v15-0012-Move-BitmapHeapScan-initialization-to-helper.patch (6.9K, ../20240405080634.d7c23f46xjhhxf5q@liskov/13-v15-0012-Move-BitmapHeapScan-initialization-to-helper.patch)
download | inline diff:
From c1c9537f045ba7293416711347c7113fa8ec33f9 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 5 Apr 2024 02:50:42 -0400
Subject: [PATCH v15 12/13] Move BitmapHeapScan initialization to helper
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 170 ++++++++++++----------
1 file changed, 92 insertions(+), 78 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index d22a433f06..aaeece6697 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -109,6 +109,94 @@ bhs_end_iterate(BitmapHeapIterator *iterator)
}
+ /*
+ * If we haven't yet performed the underlying index scan, do it, and begin
+ * the iteration over the bitmap.
+ *
+ * For prefetching, we use *two* iterators, one for the pages we are actually
+ * scanning and another that runs ahead of the first for prefetching.
+ * node->prefetch_pages tracks exactly how many pages ahead the prefetch
+ * iterator is. Also, node->prefetch_target tracks the desired prefetch
+ * distance, which starts small and increases up to the
+ * scan->prefetch_maximum. This is to avoid doing a lot of prefetching in a
+ * scan that stops after a few tuples because of a LIMIT.
+ */
+static void
+BitmapHeapInitialize(BitmapHeapScanState *node)
+{
+ Relation rel = node->ss.ss_currentRelation;
+ bool pf_maximum = 0;
+ bool init_shared_state = false;
+ uint32 extra_flags = 0;
+
+ Assert(!node->initialized);
+
+ /*
+ * The leader will immediately come out of the function, but others will
+ * be blocked until leader populates the TBM and wakes them up.
+ */
+ init_shared_state = node->pstate ?
+ BitmapShouldInitializeSharedState(node->pstate) : false;
+
+ /*
+ * Maximum number of prefetches for the tablespace if configured,
+ * otherwise the current value of the effective_io_concurrency GUC.
+ */
+#ifdef USE_PREFETCH
+ pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);
+#endif
+
+ if (!node->pstate || init_shared_state)
+ {
+ node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
+
+ if (!node->tbm || !IsA(node->tbm, TIDBitmap))
+ elog(ERROR, "unrecognized result from subplan");
+
+ if (init_shared_state)
+ {
+ /*
+ * Prepare to iterate over the TBM. This will return the
+ * dsa_pointer of the iterator state which will be used by
+ * multiple processes to iterate jointly.
+ */
+ node->pstate->tbmiterator = tbm_prepare_shared_iterate(node->tbm);
+
+#ifdef USE_PREFETCH
+ if (pf_maximum > 0)
+ {
+ node->pstate->prefetch_iterator =
+ tbm_prepare_shared_iterate(node->tbm);
+ }
+#endif
+ /* We have initialized the shared state so wake up others. */
+ BitmapDoneInitializingSharedState(node->pstate);
+ }
+ }
+
+ /*
+ * We can potentially skip fetching heap pages if we do not need any
+ * columns of the table, either for checking non-indexable quals or for
+ * returning data. This test is a bit simplistic, as it checks the
+ * stronger condition that there's no qual or return tlist at all. But in
+ * most cases it's probably not worth working harder than that.
+ */
+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)
+ extra_flags |= SO_NEED_TUPLE;
+
+ node->ss.ss_currentScanDesc = table_beginscan_bm(node->ss.ss_currentScanDesc,
+ rel,
+ node->ss.ps.state->es_snapshot,
+ extra_flags,
+ pf_maximum,
+ node->tbm,
+ node->pstate,
+ node->ss.ps.state->es_query_dsa);
+
+ node->initialized = true;
+}
+
+
/* ----------------------------------------------------------------
* BitmapHeapNext
*
@@ -124,88 +212,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* If we haven't yet performed the underlying index scan, do it, and begin
- * the iteration over the bitmap.
- *
- * For prefetching, we use *two* iterators, one for the pages we are
- * actually scanning and another that runs ahead of the first for
- * prefetching. node->prefetch_pages tracks exactly how many pages ahead
- * the prefetch iterator is. Also, node->prefetch_target tracks the
- * desired prefetch distance, which starts small and increases up to the
- * scan->prefetch_maximum. This is to avoid doing a lot of prefetching in
- * a scan that stops after a few tuples because of a LIMIT.
+ * the iteration over the bitmap. This happens on rescan as well.
*/
if (!node->initialized)
{
- Relation rel = node->ss.ss_currentRelation;
- bool pf_maximum = 0;
- bool init_shared_state = false;
- uint32 extra_flags = 0;
-
- /*
- * The leader will immediately come out of the function, but others
- * will be blocked until leader populates the TBM and wakes them up.
- */
- init_shared_state = node->pstate ?
- BitmapShouldInitializeSharedState(node->pstate) : false;
-
- /*
- * Maximum number of prefetches for the tablespace if configured,
- * otherwise the current value of the effective_io_concurrency GUC.
- */
-#ifdef USE_PREFETCH
- pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);
-#endif
-
- if (!node->pstate || init_shared_state)
- {
- node->tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
-
- if (!node->tbm || !IsA(node->tbm, TIDBitmap))
- elog(ERROR, "unrecognized result from subplan");
-
- if (init_shared_state)
- {
- /*
- * Prepare to iterate over the TBM. This will return the
- * dsa_pointer of the iterator state which will be used by
- * multiple processes to iterate jointly.
- */
- node->pstate->tbmiterator = tbm_prepare_shared_iterate(node->tbm);
-
-#ifdef USE_PREFETCH
- if (pf_maximum > 0)
- {
- node->pstate->prefetch_iterator =
- tbm_prepare_shared_iterate(node->tbm);
- }
-#endif
- /* We have initialized the shared state so wake up others. */
- BitmapDoneInitializingSharedState(node->pstate);
- }
- }
-
- /*
- * We can potentially skip fetching heap pages if we do not need any
- * columns of the table, either for checking non-indexable quals or
- * for returning data. This test is a bit simplistic, as it checks
- * the stronger condition that there's no qual or return tlist at all.
- * But in most cases it's probably not worth working harder than that.
- */
- if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)
- extra_flags |= SO_NEED_TUPLE;
-
- scan = table_beginscan_bm(node->ss.ss_currentScanDesc,
- rel,
- node->ss.ps.state->es_snapshot,
- extra_flags,
- pf_maximum,
- node->tbm,
- node->pstate,
- node->ss.ps.state->es_query_dsa);
-
- node->ss.ss_currentScanDesc = scan;
- node->initialized = true;
+ BitmapHeapInitialize(node);
+ /* We may have a new scan descriptor */
+ scan = node->ss.ss_currentScanDesc;
goto new_page;
}
--
2.40.1
[text/x-diff] v15-0013-Remove-table_scan_bitmap_next_block.patch (12.4K, ../20240405080634.d7c23f46xjhhxf5q@liskov/14-v15-0013-Remove-table_scan_bitmap_next_block.patch)
download | inline diff:
From 869023793654571cba6af0ccfd2fac9950ccc635 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 15:43:10 -0400
Subject: [PATCH v15 13/13] Remove table_scan_bitmap_next_block()
With several of the changes to the control flow of BitmapHeapNext() in
recent commits, table_scan_bitmap_next_tuple() can be responsible for
getting the next block. Do this and remove the table AM API function
table_scan_bitmap_next_block().
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 2 +
src/backend/access/heap/heapam_handler.c | 48 ++++++++-------
src/backend/access/table/tableamapi.c | 2 -
src/backend/executor/nodeBitmapHeapscan.c | 61 +++++++++----------
src/backend/optimizer/util/plancat.c | 2 +-
src/include/access/tableam.h | 74 +++++------------------
6 files changed, 73 insertions(+), 116 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a21ec92d71..9c98109a16 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -324,6 +324,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
ItemPointerSetInvalid(&scan->rs_ctup.t_self);
scan->rs_cbuf = InvalidBuffer;
scan->rs_cblock = InvalidBlockNumber;
+ scan->rs_cindex = 0;
+ scan->rs_ntuples = 0;
/* page-at-a-time fields are always invalid when not rs_inited */
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index c513cbd2da..d7f3bb960f 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2183,12 +2183,6 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
HEAP_USABLE_BYTES_PER_PAGE);
}
-
-/* ------------------------------------------------------------------------
- * Executor related callbacks for the heap AM
- * ------------------------------------------------------------------------
- */
-
/*
* BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
*
@@ -2222,8 +2216,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan)
/*
* Adjusting the prefetch iterator before invoking
- * table_scan_bitmap_next_block() keeps prefetch distance higher across
- * the parallel workers.
+ * heapam_bitmap_next_block() keeps prefetch distance higher across the
+ * parallel workers.
*/
if (scan->prefetch_maximum > 0)
{
@@ -2583,9 +2577,15 @@ BitmapPrefetch(BitmapHeapScanDesc scan)
#endif /* USE_PREFETCH */
}
+/* ------------------------------------------------------------------------
+ * Executor related callbacks for the heap AM
+ * ------------------------------------------------------------------------
+ */
+
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TupleTableSlot *slot)
+ TupleTableSlot *slot, bool *recheck,
+ long *lossy_pages, long *exact_pages)
{
BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
HeapScanDesc hscan = &bscan->heap_common;
@@ -2593,21 +2593,28 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
Page page;
ItemId lp;
- if (bscan->empty_tuples_pending > 0)
+ /*
+ * Out of range? If so, nothing more to look at on this page
+ */
+ while (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
{
/*
- * If we don't have to fetch the tuple, just return nulls.
+ * Emit empty tuples before advancing to the next block
*/
- ExecStoreAllNullTuple(slot);
- bscan->empty_tuples_pending--;
- return true;
- }
+ if (bscan->empty_tuples_pending > 0)
+ {
+ /*
+ * If we don't have to fetch the tuple, just return nulls.
+ */
+ ExecStoreAllNullTuple(slot);
+ bscan->empty_tuples_pending--;
+ return true;
+ }
- /*
- * Out of range? If so, nothing more to look at on this page
- */
- if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
- return false;
+ if (!heapam_scan_bitmap_next_block(scan, recheck,
+ lossy_pages, exact_pages))
+ return false;
+ }
#ifdef USE_PREFETCH
@@ -3008,7 +3015,6 @@ static const TableAmRoutine heapam_methods = {
.relation_estimate_size = heapam_estimate_rel_size,
- .scan_bitmap_next_block = heapam_scan_bitmap_next_block,
.scan_bitmap_next_tuple = heapam_scan_bitmap_next_tuple,
.scan_sample_next_block = heapam_scan_sample_next_block,
.scan_sample_next_tuple = heapam_scan_sample_next_tuple
diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c
index 55b8caeadf..0b87530a9c 100644
--- a/src/backend/access/table/tableamapi.c
+++ b/src/backend/access/table/tableamapi.c
@@ -90,8 +90,6 @@ GetTableAmRoutine(Oid amhandler)
Assert(routine->relation_estimate_size != NULL);
/* optional, but one callback implies presence of the other */
- Assert((routine->scan_bitmap_next_block == NULL) ==
- (routine->scan_bitmap_next_tuple == NULL));
Assert(routine->scan_sample_next_block != NULL);
Assert(routine->scan_sample_next_tuple != NULL);
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index aaeece6697..48086aae1d 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -206,58 +206,53 @@ BitmapHeapInitialize(BitmapHeapScanState *node)
static TupleTableSlot *
BitmapHeapNext(BitmapHeapScanState *node)
{
- ExprContext *econtext = node->ss.ps.ps_ExprContext;
- TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
- TableScanDesc scan = node->ss.ss_currentScanDesc;
+ ExprContext *econtext;
+ TupleTableSlot *slot;
+ TableScanDesc scan;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
* the iteration over the bitmap. This happens on rescan as well.
*/
if (!node->initialized)
- {
BitmapHeapInitialize(node);
- /* We may have a new scan descriptor */
- scan = node->ss.ss_currentScanDesc;
- goto new_page;
- }
+ /*
+ * BitmapHeapInitialize() may make the scan descriptor so don't get it
+ * from the node until after calling it.
+ */
+ econtext = node->ss.ps.ps_ExprContext;
+ slot = node->ss.ss_ScanTupleSlot;
+ scan = node->ss.ss_currentScanDesc;
- for (;;)
+ while (table_scan_bitmap_next_tuple(scan, slot, &node->recheck,
+ &node->lossy_pages, &node->exact_pages))
{
- while (table_scan_bitmap_next_tuple(scan, slot))
- {
- CHECK_FOR_INTERRUPTS();
+ CHECK_FOR_INTERRUPTS();
- /*
- * If we are using lossy info, we have to recheck the qual
- * conditions at every tuple.
- */
- if (node->recheck)
+ /*
+ * If we are using lossy info, we have to recheck the qual conditions
+ * at every tuple.
+ */
+ if (node->recheck)
+ {
+ econtext->ecxt_scantuple = slot;
+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))
{
- econtext->ecxt_scantuple = slot;
- if (!ExecQualAndReset(node->bitmapqualorig, econtext))
- {
- /* Fails recheck, so drop it and loop back for another */
- InstrCountFiltered2(node, 1);
- ExecClearTuple(slot);
- continue;
- }
+ /* Fails recheck, so drop it and loop back for another */
+ InstrCountFiltered2(node, 1);
+ ExecClearTuple(slot);
+ continue;
}
-
- /* OK to return this tuple */
- return slot;
}
-new_page:
-
- if (!table_scan_bitmap_next_block(scan, &node->recheck,
- &node->lossy_pages, &node->exact_pages))
- break;
+ /* OK to return this tuple */
+ return slot;
}
+
/*
* if we get here it means we are at the end of the scan..
*/
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 6bb53e4346..cf56cc572f 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -313,7 +313,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
info->amcanparallel = amroutine->amcanparallel;
info->amhasgettuple = (amroutine->amgettuple != NULL);
info->amhasgetbitmap = amroutine->amgetbitmap != NULL &&
- relation->rd_tableam->scan_bitmap_next_block != NULL;
+ relation->rd_tableam->scan_bitmap_next_tuple != NULL;
info->amcanmarkpos = (amroutine->ammarkpos != NULL &&
amroutine->amrestrpos != NULL);
info->amcostestimate = amroutine->amcostestimate;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 138bf5f6ed..e4d0c4b0ed 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -790,34 +790,20 @@ typedef struct TableAmRoutine
* ------------------------------------------------------------------------
*/
- /*
- * Prepare to fetch / check / return tuples as part of a bitmap table
- * scan. `scan` was started via table_beginscan_bm(). Return false if the
- * bitmap is exhausted and true otherwise.
- *
- * This will typically read and pin the target block, and do the necessary
- * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time).
- *
- * lossy_pages is incremented if the bitmap is lossy for the selected
- * block; otherwise, exact_pages is incremented.
- *
- * Optional callback, but either both scan_bitmap_next_block and
- * scan_bitmap_next_tuple need to exist, or neither.
- */
- bool (*scan_bitmap_next_block) (TableScanDesc scan,
- bool *recheck,
- long *lossy_pages, long *exact_pages);
-
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * Optional callback, but either both scan_bitmap_next_block and
- * scan_bitmap_next_tuple need to exist, or neither.
+ * recheck is set if recheck is required.
+ *
+ * The table AM is responsible for reading in blocks and counting (for
+ * EXPLAIN) which of those blocks were represented lossily in the bitmap
+ * using the lossy_pages and exact_pages counters.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- TupleTableSlot *slot);
+ TupleTableSlot *slot,
+ bool *recheck,
+ long *lossy_pages, long *exact_pages);
/*
* Prepare to fetch tuples from the next block in a sample scan. Return
@@ -1980,45 +1966,13 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples as part of a bitmap table scan.
- * `scan` needs to have been started via table_beginscan_bm(). Returns false if
- * there are no more blocks in the bitmap, true otherwise. lossy_pages is
- * incremented is the block's representation in the bitmap is lossy; otherwise,
- * exact_pages is incremented.
- *
- * Note, this is an optionally implemented function, therefore should only be
- * used after verifying the presence (at plan time or such).
- */
-static inline bool
-table_scan_bitmap_next_block(TableScanDesc scan,
- bool *recheck,
- long *lossy_pages,
- long *exact_pages)
-{
- /*
- * We don't expect direct calls to table_scan_bitmap_next_block with valid
- * CheckXidAlive for catalog or regular tables. See detailed comments in
- * xact.c where these variables are declared.
- */
- if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan))
- elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
-
- return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- recheck,
- lossy_pages, exact_pages);
-}
-
-/*
- * Fetch the next tuple of a bitmap table scan into `slot` and return true if
- * a visible tuple was found, false otherwise.
- * table_scan_bitmap_next_block() needs to previously have selected a
- * block (i.e. returned true), and no previous
- * table_scan_bitmap_next_tuple() for the same block may have
- * returned false.
+ * Fetch the next tuple of a bitmap table scan into `slot` and return true if a
+ * visible tuple was found, false otherwise.
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- TupleTableSlot *slot)
+ TupleTableSlot *slot, bool *recheck,
+ long *lossy_pages, long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_tuple with valid
@@ -2029,7 +1983,9 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- slot);
+ slot, recheck,
+ lossy_pages,
+ exact_pages);
}
/*
--
2.40.1
view thread (21+ 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]
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
In-Reply-To: <20240405080634.d7c23f46xjhhxf5q@liskov>
* 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