From 9f6b3738ac4f6648d830168d8659aa49510baecc Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 14 Mar 2024 12:39:28 -0400 Subject: [PATCH v10 09/17] 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. These changes will enable bitmapheapscan to use the future streaming read API [1]. Table AMs will implement a streaming read API callback returning the next block to fetch. In heap AM's case, the callback will use the iterator to identify the next block to fetch. Since choosing the next block will no longer the responsibility of BitmapHeapNext(), the streaming read control flow requires these changes to table_scan_bitmap_next_block(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com --- src/backend/access/heap/heapam_handler.c | 59 +++++-- src/backend/executor/nodeBitmapHeapscan.c | 198 ++++++++++------------ src/include/access/relscan.h | 7 + src/include/access/tableam.h | 68 +++++--- src/include/nodes/execnodes.h | 12 +- 5 files changed, 195 insertions(+), 149 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index cf4387f443..2ad785e511 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2114,18 +2114,51 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, static bool heapam_scan_bitmap_next_block(TableScanDesc scan, - TBMIterateResult *tbmres, - bool *lossy) + bool *recheck, bool *lossy, BlockNumber *blockno) { 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 @@ -2144,16 +2177,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. @@ -2245,7 +2269,14 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, *lossy = tbmres->ntuples < 0; - 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 282dcb9791..7e73583fe5 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); @@ -70,8 +69,8 @@ BitmapHeapNext(BitmapHeapScanState *node) { ExprContext *econtext; TableScanDesc scan; + bool lossy; TIDBitmap *tbm; - TBMIterateResult *tbmres; TupleTableSlot *slot; ParallelBitmapHeapState *pstate = node->pstate; dsa_area *dsa = node->ss.ps.state->es_query_dsa; @@ -83,7 +82,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 +109,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 +161,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) @@ -202,55 +198,19 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } - node->tbmiterator = tbmiterator; - node->shared_tbmiterator = shared_tbmiterator; + scan->tbmiterator = tbmiterator; + scan->shared_tbmiterator = shared_tbmiterator; + node->initialized = true; + + goto new_page; } for (;;) { - bool valid, lossy; - - 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); - - valid = table_scan_bitmap_next_block(scan, tbmres, &lossy); - - if (lossy) - node->lossy_pages++; - else - node->exact_pages++; - - if (!valid) - { - /* 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 @@ -272,45 +232,60 @@ 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->recheck, &lossy, &node->blockno)) + break; + + if (lossy) + node->lossy_pages++; + else + node->exact_pages++; + /* - * 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); } /* @@ -336,13 +311,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) { @@ -356,14 +335,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; @@ -388,7 +370,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 */ @@ -466,6 +451,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, @@ -523,6 +509,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 && @@ -584,12 +572,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) @@ -597,13 +581,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); @@ -634,28 +618,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); - } /* ---------------------------------------------------------------- @@ -688,8 +668,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; @@ -697,9 +675,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 bcf1497f67..a820cc8c99 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -21,6 +21,7 @@ #include "access/sdir.h" #include "access/xact.h" #include "executor/tuptable.h" +#include "nodes/tidbitmap.h" #include "utils/rel.h" #include "utils/snapshot.h" @@ -788,19 +789,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 indicates whether or not the block's representation in the bitmap * is lossy or exact. * @@ -819,8 +815,8 @@ typedef struct TableAmRoutine * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_block) (TableScanDesc scan, - struct TBMIterateResult *tbmres, - bool *lossy); + bool *recheck, bool *lossy, + BlockNumber *blockno); /* * Fetch the next tuple of a bitmap table scan into `slot` and return true @@ -957,9 +953,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; } /* @@ -1019,6 +1019,21 @@ table_beginscan_analyze(Relation rel) 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); } @@ -1029,6 +1044,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); } @@ -1981,19 +2011,17 @@ 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 is set to true if bitmap is lossy for the - * selected block and false otherwise. + * 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 is set to true + * if bitmap is lossy for the selected block and false otherwise. * * 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, - bool *lossy) + bool *recheck, bool *lossy, BlockNumber *blockno) { /* * We don't expect direct calls to table_scan_bitmap_next_block with valid @@ -2003,8 +2031,8 @@ table_scan_bitmap_next_block(TableScanDesc scan, 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, - tbmres, lossy); + return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan, recheck, + lossy, blockno); } /* diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 6871db9b21..8688bc5ab0 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1783,8 +1783,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 @@ -1793,9 +1791,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 @@ -1803,8 +1803,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; @@ -1813,9 +1811,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