Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rpZ6D-007MDo-8q for pgsql-hackers@arkaria.postgresql.org; Wed, 27 Mar 2024 19:38:14 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1rpZ6A-001gXx-QE for pgsql-hackers@arkaria.postgresql.org; Wed, 27 Mar 2024 19:38:10 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rpZ69-001gXe-M9 for pgsql-hackers@lists.postgresql.org; Wed, 27 Mar 2024 19:38:10 +0000 Received: from mail-vs1-xe2e.google.com ([2607:f8b0:4864:20::e2e]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1rpZ5u-006kp6-9j for pgsql-hackers@postgresql.org; Wed, 27 Mar 2024 19:38:08 +0000 Received: by mail-vs1-xe2e.google.com with SMTP id ada2fe7eead31-472967264e7so57981137.3 for ; Wed, 27 Mar 2024 12:37:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711568274; x=1712173074; darn=postgresql.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=SMRVFZZCmOn/7uld4Cf4aimRb8IL21+VKE6v3N6VMa4=; b=X66sjWc+Xv1wfVu69XVR98B5nUoazbNGH0Zs5nll+gMI4vnOY/9b0DPcv74V3I7I7I yZSE4cWfV+2u5qPwUxBlStJxNpx3N/v2O8K0sBa2zxMqxp4fVkKNHoKwFlmz2u/rRmgj lYO1jyZNdofb6b9xE+OrLq72eltBPUlsO/8tiRnnREbmy9/HAxwE1I000Zs0PfeRUdOM pn+dPYERk4EVEVzrpdcFWy9njHsMjhrzGyCy8ml4WDRC3BKhfIBagbSegFsrY3SqE6zf CKIQdSRzdfsNIde6gRaORpSg0VlEbhKN6DRSw2mRreaAEMCcJfCe4Q7nyGGiqadBtLoF Syaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711568274; x=1712173074; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=SMRVFZZCmOn/7uld4Cf4aimRb8IL21+VKE6v3N6VMa4=; b=vQMvs0d8Ru4rT+75c1Rm4aNnZPXde1RYQaIabfcvEPcW3ZlN73suad/fwRITqHAjwP dqta4558Wn2nW1aj97WMICJmFb6VTiOnuBFiBSbHAhsobSQYpQ/Ln/q51yE+uzFEgWD2 DNV4SLB/RwotPAJCc4AUXSGWv0OhakAygVW8l3JEP3ymZJ3XlycrGWDu4NDruPXEmpFK o7g6qM6c7D7p3/tyEW0EtdlGnS/xkQIvy+gqGHLLy1fpzVmosKycMWHH843TI9iobgjF PFyD9q8bwu6qaL7R3diuqN0NtJZisTIisQKyQMU6T1lthi1Bwh36CZQvH3gu62sDnMs7 tNpw== X-Forwarded-Encrypted: i=1; AJvYcCW42aOCiId3ESnDwh26YsVF4C4wnixqZCF1oO8FlMR9NRW6168J10bV35/MYq/Qu5hZaB6JGxOlr4jHsseUcMKbrG73UaJlyhMX2v47 X-Gm-Message-State: AOJu0Ywq9U3nws2ydfZjtSVPNGSibZj9U99z78YNfBTy2QvNcW5HsP84 CCYyDtVLodQkeldwE2sE8kuQj+6v4voOnowmbGGHVwLGbdmvcn/9 X-Google-Smtp-Source: AGHT+IFcS7pbzKzzeQqY+3AHzE0qWse061V6hQwPbntn50CmEmd/BUyTHmkIIOavjtqx8AmnS/7ljg== X-Received: by 2002:a05:6102:3bed:b0:478:44db:f84d with SMTP id be13-20020a0561023bed00b0047844dbf84dmr309867vsb.18.1711568273506; Wed, 27 Mar 2024 12:37:53 -0700 (PDT) Received: from liskov ([151.197.204.27]) by smtp.gmail.com with ESMTPSA id gh14-20020a05621429ce00b006913aa64629sm6558096qvb.22.2024.03.27.12.37.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Mar 2024 12:37:52 -0700 (PDT) Date: Wed, 27 Mar 2024 15:37:50 -0400 From: Melanie Plageman To: Tomas Vondra Cc: Heikki Linnakangas , Andres Freund , Pg Hackers , Thomas Munro , Nazir Bilal Yavuz Subject: Re: BitmapHeapScan streaming read user and prelim refactoring Message-ID: <20240327193750.3mlcmzqondpj27xe@liskov> References: <5a172d1e-d69c-409a-b1fa-6521214c81c2@iki.fi> <20240323002211.on5vb5ulk6lsdb2u@liskov> <20240323002607.abegehmmjyxrnmm2@liskov> <20240324173833.rdtw42aur2q5hhb2@liskov> <58fa8dd3-a49b-4774-a9c4-7422ba8bc698@enterprisedb.com> <358c737e-772c-40dd-9ecf-35b0c05828cd@enterprisedb.com> <20240325160709.qjo6txzu6zjxzkqy@liskov> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="owzzsiozz6hgpp7e" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240325160709.qjo6txzu6zjxzkqy@liskov> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --owzzsiozz6hgpp7e Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Mon, Mar 25, 2024 at 12:07:09PM -0400, Melanie Plageman wrote: > On Sun, Mar 24, 2024 at 06:37:20PM -0400, Melanie Plageman wrote: > > On Sun, Mar 24, 2024 at 5:59 PM Tomas Vondra > > wrote: > > > > > > BTW when you say "up to 'Make table_scan_bitmap_next_block() async > > > friendly'" do you mean including that patch, or that this is the first > > > patch that is not one of the independently useful patches. > > > > I think the code is easier to understand with "Make > > table_scan_bitmap_next_block() async friendly". Prior to that commit, > > table_scan_bitmap_next_block() could return false even when the bitmap > > has more blocks and expects the caller to handle this and invoke it > > again. I think that interface is very confusing. The downside of the > > code in that state is that the code for prefetching is still in the > > BitmapHeapNext() code and the code for getting the current block is in > > the heap AM-specific code. I took a stab at fixing this in v9's 0013, > > but the outcome wasn't very attractive. > > > > What I will do tomorrow is reorder and group the commits such that all > > of the commits that are useful independent of streaming read are first > > (I think 0014 and 0015 are independently valuable but they are on top > > of some things that are only useful to streaming read because they are > > more recently requested changes). I think I can actually do a bit of > > simplification in terms of how many commits there are and what is in > > each. Just to be clear, v9 is still reviewable. I am just going to go > > back and change what is included in each commit. > > So, attached v10 does not include the new version of streaming read API. > I focused instead on the refactoring patches commit regrouping I > mentioned here. Attached v11 has the updated Read Stream API Thomas sent this morning [1]. No other changes. - Melanie [1] https://www.postgresql.org/message-id/CA%2BhUKGJTwrS7F%3DuJPx3SeigMiQiW%2BLJaOkjGyZdCntwyMR%3DuAw%40mail.gmail.com --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch" From c8bdc1f2143adb2d9a5c9f8f69c249b307287a1a Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 18:50:29 -0500 Subject: [PATCH v11 01/17] BitmapHeapScan begin scan after bitmap creation There is no reason for a BitmapHeapScan to begin the scan of the underlying table in ExecInitBitmapHeapScan(). Instead, do so after completing the index scan and building the bitmap. --- src/backend/access/table/tableam.c | 1 - src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 8d3675be95..a254e3175b 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key) NULL, flags); } - /* ---------------------------------------------------------------------------- * Parallel table scan related functions. * ---------------------------------------------------------------------------- diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index cee7f45aab..93fdcd226b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -178,6 +178,20 @@ 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 = node->ss.ss_currentScanDesc = table_beginscan_bm( + node->ss.ss_currentRelation, + node->ss.ps.state->es_snapshot, + 0, + NULL); + } + node->initialized = true; } @@ -601,7 +615,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 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * close heap scan */ - table_endscan(scanDesc); + if (scanDesc) + table_endscan(scanDesc); + } /* ---------------------------------------------------------------- @@ -783,11 +800,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0002-BitmapHeapScan-set-can_skip_fetch-later.patch" From 6e41737489dbeb07bc9f0cf9c595c11115cb985b Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 13 Feb 2024 14:38:41 -0500 Subject: [PATCH v11 02/17] BitmapHeapScan set can_skip_fetch later Set BitmapHeapScanState->can_skip_fetch in BitmapHeapNext() when !BitmapHeapScanState->initialized instead of in ExecInitBitmapHeapScan(). This is a preliminary step to removing can_skip_fetch from BitmapHeapScanState and setting it in table AM specific code. --- 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 93fdcd226b..c64530674b 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)); @@ -742,16 +752,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0003-Push-BitmapHeapScan-skip-fetch-optimization-into.patch" From 523d1a454c77ccb8734a16b02338145deacd0a60 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 20:15:05 -0500 Subject: [PATCH v11 03/17] 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 and implement it for the heap table AM by moving the vmbuffer and other fields needed for the optimization from the BitmapHeapScanState into the HeapScanDescData. heapam_scan_bitmap_next_block() now decides whether or not to skip fetching the block before reading it in and heapam_scan_bitmap_next_tuple() returns NULL-filled tuples for skipped blocks. The layering violation is still present in BitmapHeapScans's prefetching code. However, this will be eliminated when prefetching is implemented using the upcoming streaming read API discussed in [1]. [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%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 2f6527df0d..ed3a3607b7 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -948,6 +948,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. @@ -1036,6 +1038,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 */ @@ -1055,6 +1063,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 6abfe36dec..5ba8cb3657 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" @@ -2182,6 +2183,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 @@ -2294,6 +2313,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 c64530674b..83d9db8f39 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,11 +185,25 @@ 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 = node->ss.ss_currentScanDesc = table_beginscan_bm( node->ss.ss_currentRelation, node->ss.ps.state->es_snapshot, 0, - NULL); + NULL, + extra_flags); } node->initialized = true; @@ -207,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node) for (;;) { - bool skip_fetch; + bool valid; CHECK_FOR_INTERRUPTS(); @@ -228,37 +232,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; @@ -301,52 +282,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 */ @@ -518,7 +480,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, @@ -569,7 +531,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, @@ -639,8 +601,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; @@ -650,7 +610,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); @@ -695,8 +654,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); @@ -740,8 +697,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; @@ -752,7 +707,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 f112245373..c7a538221a 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -72,6 +72,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 fc0e702715..ae758857bd 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -62,6 +62,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; /* @@ -963,9 +970,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 1774c56ae3..6871db9b21 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1785,10 +1785,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 @@ -1808,9 +1805,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0004-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch" From 54afa2d047bd10d451c7120ec33e476974534219 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 19:03:24 -0500 Subject: [PATCH v11 04/17] 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. It makes more sense for it to use the recheck flag from the TBMIterateResult for the prefetch block instead. See this [1] thread on hackers reporting the issue. [1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%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 83d9db8f39..5df3b5ca46 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -474,14 +474,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)); @@ -532,7 +527,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" From 227a1ea3d6ceeecc57538a3a4ecb4839fd769445 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 19:04:48 -0500 Subject: [PATCH v11 05/17] 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. --- 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 5df3b5ca46..404de0595e 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); @@ -230,7 +230,7 @@ BitmapHeapNext(BitmapHeapScanState *node) break; } - BitmapAdjustPrefetchIterator(node, tbmres); + BitmapAdjustPrefetchIterator(node, tbmres->blockno); valid = table_scan_bitmap_next_block(scan, tbmres); @@ -341,7 +341,7 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate) */ static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node, - TBMIterateResult *tbmres) + BlockNumber blockno) { #ifdef USE_PREFETCH ParallelBitmapHeapState *pstate = node->pstate; @@ -360,7 +360,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0006-table_scan_bitmap_next_block-returns-lossy-or-ex.patch" From 42ad33d1f16aeb9f763a896f8677254292f9b4c2 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 26 Feb 2024 20:34:07 -0500 Subject: [PATCH v11 06/17] table_scan_bitmap_next_block() returns lossy or exact Future commits will remove the TBMIterateResult from BitmapHeapNext() -- pushing it into the table AM-specific code. So, the table AM must inform BitmapHeapNext() whether or not the current block is lossy or exact for the purposes of the counters used in EXPLAIN. --- src/backend/access/heap/heapam_handler.c | 5 ++++- src/backend/executor/nodeBitmapHeapscan.c | 10 +++++----- src/include/access/tableam.h | 14 ++++++++++---- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 5ba8cb3657..ce47a158ae 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2172,7 +2172,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, static bool heapam_scan_bitmap_next_block(TableScanDesc scan, - TBMIterateResult *tbmres) + TBMIterateResult *tbmres, + bool *lossy) { HeapScanDesc hscan = (HeapScanDesc) scan; BlockNumber block = tbmres->blockno; @@ -2300,6 +2301,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, Assert(ntup <= MaxHeapTuplesPerPage); hscan->rs_ntuples = ntup; + *lossy = tbmres->ntuples < 0; + return ntup > 0; } diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 404de0595e..c95e3412da 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -211,7 +211,7 @@ BitmapHeapNext(BitmapHeapScanState *node) for (;;) { - bool valid; + bool valid, lossy; CHECK_FOR_INTERRUPTS(); @@ -232,12 +232,12 @@ BitmapHeapNext(BitmapHeapScanState *node) BitmapAdjustPrefetchIterator(node, tbmres->blockno); - valid = table_scan_bitmap_next_block(scan, tbmres); + valid = table_scan_bitmap_next_block(scan, tbmres, &lossy); - if (tbmres->ntuples >= 0) - node->exact_pages++; - else + if (lossy) node->lossy_pages++; + else + node->exact_pages++; if (!valid) { diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index ae758857bd..5c2a7b7422 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -815,6 +815,9 @@ typedef struct TableAmRoutine * 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. + * * 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 @@ -830,7 +833,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, + bool *lossy); /* * Fetch the next tuple of a bitmap table scan into `slot` and return true @@ -2023,14 +2027,16 @@ 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. + * the page, 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) + struct TBMIterateResult *tbmres, + bool *lossy) { /* * We don't expect direct calls to table_scan_bitmap_next_block with valid @@ -2041,7 +2047,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, - tbmres); + tbmres, lossy); } /* -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0007-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch" From 8fec5f876636383e802dbb10c107892bb71a4d8d Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 13 Feb 2024 10:17:47 -0500 Subject: [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables To simplify the diff of a future commit which will move the TBMIterators into the scan descriptor, define them in a narrower scope now. --- 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 c95e3412da..49938c9ed4 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 @@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } + node->tbmiterator = tbmiterator; + node->shared_tbmiterator = shared_tbmiterator; node->initialized = true; } @@ -221,9 +219,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch" From 836b7922f3d2c85472525d22304436409f06fd90 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 12 Feb 2024 18:13:41 -0500 Subject: [PATCH v11 08/17] Remove table_scan_bitmap_next_tuple parameter tbmres With the addition of the proposed streaming read API [1], table_scan_bitmap_next_block() will no longer take a TBMIterateResult as an input. Instead table AMs will be responsible for implementing a callback for the streaming read API which specifies which blocks should be prefetched and read. Thus, it no longer makes sense to use the TBMIterateResult 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(). [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%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 ce47a158ae..ddcdbbaf7e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2308,7 +2308,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 49938c9ed4..282dcb9791 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -286,7 +286,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 5c2a7b7422..9c7b8bf162 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -806,10 +806,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 @@ -840,15 +837,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); /* @@ -2060,7 +2052,6 @@ table_scan_bitmap_next_block(TableScanDesc scan, */ static inline bool table_scan_bitmap_next_tuple(TableScanDesc scan, - struct TBMIterateResult *tbmres, TupleTableSlot *slot) { /* @@ -2072,7 +2063,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0009-Make-table_scan_bitmap_next_block-async-friendly.patch" From 9affa3100a127c747608e3cd8726696aabe93530 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 14 Mar 2024 12:39:28 -0400 Subject: [PATCH v11 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 ddcdbbaf7e..196f69e30e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2172,18 +2172,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 @@ -2202,16 +2235,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. @@ -2303,7 +2327,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 9c7b8bf162..68478d16b2 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" @@ -799,19 +800,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. * @@ -830,8 +826,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 @@ -968,9 +964,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; } /* @@ -1030,6 +1030,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); } @@ -1040,6 +1055,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); } @@ -2016,19 +2046,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 @@ -2038,8 +2066,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0010-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch" From 3f3bb4559e8a9862fb6c56c8de0c2e4abc69e5c2 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 25 Mar 2024 11:05:51 -0400 Subject: [PATCH v11 10/17] 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(). Now, on the first scan, the the iterator is created after the scan descriptor is created. --- src/backend/access/heap/heapam_handler.c | 5 +- src/backend/executor/nodeBitmapHeapscan.c | 163 ++++++++++++---------- src/include/access/relscan.h | 7 +- src/include/access/tableam.h | 29 +--- src/include/executor/nodeBitmapHeapscan.h | 10 ++ src/include/nodes/execnodes.h | 8 +- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 116 insertions(+), 107 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 196f69e30e..2d9e0e1a9f 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2191,10 +2191,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 7e73583fe5..fe471a8a0c 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -56,6 +56,56 @@ static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node); static inline void BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan); static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate); +static BitmapHeapIterator *bhs_begin_iterate(TIDBitmap *tbm, + dsa_pointer shared_area, + dsa_area *personal_area); + +BitmapHeapIterator * +bhs_begin_iterate(TIDBitmap *tbm, dsa_pointer shared_area, dsa_area *personal_area) +{ + BitmapHeapIterator *result = palloc(sizeof(BitmapHeapIterator)); + + result->serial = NULL; + result->parallel = NULL; + + /* Allocate a private iterator and attach the shared state to it */ + if (DsaPointerIsValid(shared_area)) + result->parallel = tbm_attach_shared_iterate(personal_area, shared_area); + else + result->serial = tbm_begin_iterate(tbm); + + return result; +} + +TBMIterateResult * +bhs_iterate(BitmapHeapIterator *iterator) +{ + Assert(iterator); + + if (iterator->serial) + return tbm_iterate(iterator->serial); + else + return tbm_shared_iterate(iterator->parallel); +} + +void +bhs_end_iterate(BitmapHeapIterator *iterator) +{ + Assert(iterator); + + if (iterator->serial) + { + tbm_end_iterate(iterator->serial); + iterator->serial = NULL; + } + else + { + tbm_end_shared_iterate(iterator->parallel); + iterator->parallel = NULL; + } + + pfree(iterator); +} /* ---------------------------------------------------------------- @@ -97,43 +147,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) - { - 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)) + if (init_shared_state) { - 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 @@ -154,21 +184,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 +216,21 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } - scan->tbmiterator = tbmiterator; - scan->shared_tbmiterator = shared_tbmiterator; + scan->rs_bhs_iterator = bhs_begin_iterate(tbm, + pstate ? pstate->tbmiterator : InvalidDsaPointer, + dsa); + +#ifdef USE_PREFETCH + if (node->prefetch_maximum > 0) + { + node->pf_iterator = bhs_begin_iterate(tbm, + pstate ? pstate->prefetch_iterator : InvalidDsaPointer, + dsa); + /* Only used for serial BHS */ + node->prefetch_pages = 0; + node->prefetch_target = -1; + } +#endif /* USE_PREFETCH */ node->initialized = true; @@ -280,7 +311,7 @@ new_page: * ahead of the current block. */ if (node->pstate == NULL && - node->prefetch_iterator && + node->pf_iterator && node->pfblockno > node->blockno) elog(ERROR, "prefetch and main iterators are out of sync"); @@ -321,12 +352,11 @@ 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 */ @@ -335,7 +365,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) else if (prefetch_iterator) { /* 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; @@ -348,8 +378,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (node->prefetch_maximum > 0) { - TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator; - SpinLockAcquire(&pstate->mutex); if (pstate->prefetch_pages > 0) { @@ -371,7 +399,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (prefetch_iterator) { - tbmpre = tbm_shared_iterate(prefetch_iterator); + tbmpre = bhs_iterate(prefetch_iterator); node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; } } @@ -431,23 +459,22 @@ 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) { 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); + node->pf_iterator = NULL; break; } node->prefetch_pages++; @@ -475,8 +502,6 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) if (pstate->prefetch_pages < pstate->prefetch_target) { - TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator; - if (prefetch_iterator) { while (1) @@ -500,12 +525,12 @@ 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); + node->pf_iterator = NULL; break; } @@ -572,18 +597,17 @@ 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) + { + bhs_end_iterate(node->pf_iterator); + node->pf_iterator = NULL; + } 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; @@ -628,12 +652,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * release bitmaps and buffers if any */ - if (node->prefetch_iterator) - tbm_end_iterate(node->prefetch_iterator); + if (node->pf_iterator) + 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); } @@ -671,11 +693,10 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->pvmbuffer = InvalidBuffer; scanstate->exact_pages = 0; scanstate->lossy_pages = 0; - scanstate->prefetch_iterator = NULL; + scanstate->pf_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..fb22f305bf 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -20,12 +20,12 @@ #include "storage/buf.h" #include "storage/spin.h" #include "utils/relcache.h" +#include "executor/nodeBitmapHeapscan.h" struct ParallelTableScanDescData; -struct TBMIterator; -struct TBMSharedIterator; +struct BitmapHeapIterator; /* * Generic descriptor for table scans. This is the base-class for table scans, @@ -44,8 +44,7 @@ typedef struct TableScanDescData ItemPointerData rs_maxtid; /* Only used for Bitmap table scans */ - struct TBMIterator *tbmiterator; - struct TBMSharedIterator *shared_tbmiterator; + struct 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 68478d16b2..459e123e92 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -968,8 +968,7 @@ table_beginscan_bm(Relation rel, Snapshot snapshot, 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; + result->rs_bhs_iterator = NULL; return result; } @@ -1032,17 +1031,8 @@ 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; - } + bhs_end_iterate(scan->rs_bhs_iterator); + scan->rs_bhs_iterator = NULL; } scan->rs_rd->rd_tableam->scan_end(scan); @@ -1057,17 +1047,8 @@ table_rescan(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; - } + bhs_end_iterate(scan->rs_bhs_iterator); + scan->rs_bhs_iterator = NULL; } 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..cb56d20dc6 100644 --- a/src/include/executor/nodeBitmapHeapscan.h +++ b/src/include/executor/nodeBitmapHeapscan.h @@ -28,5 +28,15 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node, ParallelContext *pcxt); extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt); +typedef struct BitmapHeapIterator +{ + struct TBMIterator *serial; + struct TBMSharedIterator *parallel; +} BitmapHeapIterator; + +extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator); + +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 8688bc5ab0..52cedd1b35 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1778,6 +1778,8 @@ typedef struct ParallelBitmapHeapState ConditionVariable cv; } ParallelBitmapHeapState; +struct BitmapHeapIterator; + /* ---------------- * BitmapHeapScanState information * @@ -1786,12 +1788,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 @@ -1806,12 +1807,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; + struct BitmapHeapIterator *pf_iterator; ParallelBitmapHeapState *pstate; bool recheck; BlockNumber blockno; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index cfa9d5aaea..a6562d19a6 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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0011-table_scan_bitmap_next_block-counts-lossy-and-ex.patch" From 5621e637fd815e6c7fd5ee492678c943cdac82ea Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 17:09:12 -0400 Subject: [PATCH v11 11/17] table_scan_bitmap_next_block counts lossy and exact pages Now that the table_scan_bitmap_next_block() callback only returns false when the bitmap is exhausted, it is simpler to move the management of the lossy and exact page counters into it. We will eventually remove this callback and table_scan_bitmap_next_tuple() will update those counters when a new block is read in. --- src/backend/access/heap/heapam_handler.c | 8 ++++++-- src/backend/executor/nodeBitmapHeapscan.c | 9 ++------- src/include/access/tableam.h | 21 +++++++++++++-------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 2d9e0e1a9f..81a7488007 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2172,7 +2172,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, static bool heapam_scan_bitmap_next_block(TableScanDesc scan, - bool *recheck, bool *lossy, BlockNumber *blockno) + bool *recheck, BlockNumber *blockno, + long *lossy_pages, long *exact_pages) { HeapScanDesc hscan = (HeapScanDesc) scan; BlockNumber block; @@ -2322,7 +2323,10 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, Assert(ntup <= MaxHeapTuplesPerPage); hscan->rs_ntuples = ntup; - *lossy = tbmres->ntuples < 0; + if (tbmres->ntuples < 0) + (*lossy_pages)++; + else + (*exact_pages)++; /* * Return true to indicate that a valid block was found and the bitmap is diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index fe471a8a0c..076e1ff674 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -119,7 +119,6 @@ BitmapHeapNext(BitmapHeapScanState *node) { ExprContext *econtext; TableScanDesc scan; - bool lossy; TIDBitmap *tbm; TupleTableSlot *slot; ParallelBitmapHeapState *pstate = node->pstate; @@ -298,14 +297,10 @@ new_page: BitmapAdjustPrefetchIterator(node); - if (!table_scan_bitmap_next_block(scan, &node->recheck, &lossy, &node->blockno)) + if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno, + &node->lossy_pages, &node->exact_pages)) break; - if (lossy) - node->lossy_pages++; - else - node->exact_pages++; - /* * If serial, we can error out if the the prefetch block doesn't stay * ahead of the current block. diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 459e123e92..bb2b79717c 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -808,8 +808,8 @@ typedef struct TableAmRoutine * 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 indicates whether or not the block's representation in the bitmap - * is lossy or exact. + * lossy_pages is incremented if the block's representation in the bitmap + * is lossy, 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 @@ -826,8 +826,10 @@ typedef struct TableAmRoutine * scan_bitmap_next_tuple need to exist, or neither. */ bool (*scan_bitmap_next_block) (TableScanDesc scan, - bool *recheck, bool *lossy, - BlockNumber *blockno); + bool *recheck, + BlockNumber *blockno, + long *lossy_pages, + long *exact_pages); /* * Fetch the next tuple of a bitmap table scan into `slot` and return true @@ -2029,15 +2031,17 @@ 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 is set to true - * if bitmap is lossy for the selected block and false otherwise. + * there are no more blocks in the bitmap, true otherwise. lossy_pages is + * incremented if bitmap is lossy for the selected block and exact_pages is + * incremented 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, - bool *recheck, bool *lossy, BlockNumber *blockno) + bool *recheck, BlockNumber *blockno, + long *lossy_pages, long *exact_pages) { /* * We don't expect direct calls to table_scan_bitmap_next_block with valid @@ -2048,7 +2052,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, recheck, - lossy, blockno); + blockno, lossy_pages, + exact_pages); } /* -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0012-Hard-code-TBMIterateResult-offsets-array-size.patch" From f457af280f8d84c43e4f6cf5a0cbd76bb06f5f19 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 15 Feb 2024 20:13:43 -0500 Subject: [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets but the API always allocated MaxHeapTuplesPerPage OffsetNumbers. Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear for the API user. --- src/backend/nodes/tidbitmap.c | 33 +++++++-------------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fc..1dc4c99bf9 100644 --- a/src/backend/nodes/tidbitmap.c +++ b/src/backend/nodes/tidbitmap.c @@ -40,7 +40,6 @@ #include -#include "access/htup_details.h" #include "common/hashfn.h" #include "common/int.h" #include "nodes/bitmapset.h" @@ -48,14 +47,6 @@ #include "storage/lwlock.h" #include "utils/dsa.h" -/* - * The maximum number of tuples per page is not large (typically 256 with - * 8K pages, or 1024 with 32K pages). So there's not much point in making - * the per-page bitmaps variable size. We just legislate that the size - * is this: - */ -#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage - /* * When we have to switch over to lossy storage, we use a data structure * with one bit per page, where all pages having the same number DIV @@ -67,7 +58,7 @@ * table, using identical data structures. (This is because the memory * management for hashtables doesn't easily/efficiently allow space to be * transferred easily from one hashtable to another.) Therefore it's best - * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not + * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not * too different. But we also want PAGES_PER_CHUNK to be a power of 2 to * avoid expensive integer remainder operations. So, define it like this: */ @@ -79,7 +70,7 @@ #define BITNUM(x) ((x) % BITS_PER_BITMAPWORD) /* number of active words for an exact page: */ -#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1) +#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1) /* number of active words for a lossy chunk: */ #define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1) @@ -181,7 +172,7 @@ struct TBMIterator int spageptr; /* next spages index */ int schunkptr; /* next schunks index */ int schunkbit; /* next bit to check in current schunk */ - TBMIterateResult output; /* MUST BE LAST (because variable-size) */ + TBMIterateResult output; }; /* @@ -222,7 +213,7 @@ struct TBMSharedIterator PTEntryArray *ptbase; /* pagetable element array */ PTIterationArray *ptpages; /* sorted exact page index list */ PTIterationArray *ptchunks; /* sorted lossy page index list */ - TBMIterateResult output; /* MUST BE LAST (because variable-size) */ + TBMIterateResult output; }; /* Local function prototypes */ @@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids, bitnum; /* safety check to ensure we don't overrun bit array bounds */ - if (off < 1 || off > MAX_TUPLES_PER_PAGE) + if (off < 1 || off > MaxHeapTuplesPerPage) elog(ERROR, "tuple offset out of range: %u", off); /* @@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm) Assert(tbm->iterating != TBM_ITERATING_SHARED); - /* - * Create the TBMIterator struct, with enough trailing space to serve the - * needs of the TBMIterateResult sub-struct. - */ - iterator = (TBMIterator *) palloc(sizeof(TBMIterator) + - MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber)); + iterator = palloc(sizeof(TBMIterator)); iterator->tbm = tbm; /* @@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) TBMSharedIterator *iterator; TBMSharedIteratorState *istate; - /* - * Create the TBMSharedIterator struct, with enough trailing space to - * serve the needs of the TBMIterateResult sub-struct. - */ - iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) + - MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber)); + iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator)); istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp); diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h index 1945f0639b..432fae5296 100644 --- a/src/include/nodes/tidbitmap.h +++ b/src/include/nodes/tidbitmap.h @@ -22,6 +22,7 @@ #ifndef TIDBITMAP_H #define TIDBITMAP_H +#include "access/htup_details.h" #include "storage/itemptr.h" #include "utils/dsa.h" @@ -41,9 +42,16 @@ typedef struct TBMIterateResult { BlockNumber blockno; /* page number containing tuples */ int ntuples; /* -1 indicates lossy result */ - bool recheck; /* should the tuples be rechecked? */ /* Note: recheck is always true if ntuples < 0 */ - OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; + bool recheck; /* should the tuples be rechecked? */ + + /* + * The maximum number of tuples per page is not large (typically 256 with + * 8K pages, or 1024 with 32K pages). So there's not much point in making + * the per-page bitmaps variable size. We just legislate that the size is + * this: + */ + OffsetNumber offsets[MaxHeapTuplesPerPage]; } TBMIterateResult; /* function prototypes in nodes/tidbitmap.c */ -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" From 7fa873c6bc40682f58193d8bc00848d5e2f5cb16 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 15 Feb 2024 21:23:41 -0500 Subject: [PATCH v11 13/17] Separate TBM[Shared]Iterator and TBMIterateResult Remove the TBMIterateResult from the TBMIterator and TBMSharedIterator and have tbm_[shared_]iterate() take a TBMIterateResult as a parameter. This will allow multiple TBMIterateResults to exist concurrently allowing asynchronous use of the TIDBitmap for prefetching, for example. tbm_[shared]_iterate() now sets blockno to InvalidBlockNumber when the bitmap is exhausted instead of returning NULL. BitmapHeapScan callers of tbm_iterate make a TBMIterateResult locally and pass it in. Because GIN only needs a single TBMIterateResult, inline the matchResult in the GinScanEntry to avoid having to separately manage memory for the TBMIterateResult. --- src/backend/access/gin/ginget.c | 48 +++++++++------ src/backend/access/gin/ginscan.c | 2 +- src/backend/access/heap/heapam_handler.c | 30 +++++----- src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++------- src/backend/nodes/tidbitmap.c | 73 ++++++++++++----------- src/include/access/gin_private.h | 2 +- src/include/executor/nodeBitmapHeapscan.h | 2 +- src/include/nodes/tidbitmap.h | 4 +- 8 files changed, 113 insertions(+), 95 deletions(-) diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c index 0b4f2ebadb..3aa457a29e 100644 --- a/src/backend/access/gin/ginget.c +++ b/src/backend/access/gin/ginget.c @@ -332,10 +332,22 @@ restartScanEntry: entry->list = NULL; entry->nlist = 0; entry->matchBitmap = NULL; - entry->matchResult = NULL; entry->reduceResult = false; entry->predictNumberResult = 0; + /* + * MTODO: is it enough to set blockno to InvalidBlockNumber? In all the + * places were we previously set matchResult to NULL, I just set blockno + * to InvalidBlockNumber. It seems like this should be okay because that + * is usually what we check before using the matchResult members. But it + * might be safer to zero out the offsets array. But that is expensive. + */ + entry->matchResult.blockno = InvalidBlockNumber; + entry->matchResult.ntuples = 0; + entry->matchResult.recheck = true; + memset(entry->matchResult.offsets, 0, + sizeof(OffsetNumber) * MaxHeapTuplesPerPage); + /* * we should find entry, and begin scan of posting tree or just store * posting list in memory @@ -374,6 +386,7 @@ restartScanEntry: { if (entry->matchIterator) tbm_end_iterate(entry->matchIterator); + entry->matchResult.blockno = InvalidBlockNumber; entry->matchIterator = NULL; tbm_free(entry->matchBitmap); entry->matchBitmap = NULL; @@ -823,18 +836,19 @@ entryGetItem(GinState *ginstate, GinScanEntry entry, { /* * If we've exhausted all items on this block, move to next block - * in the bitmap. + * in the bitmap. tbm_iterate() sets matchResult->blockno to + * InvalidBlockNumber when the bitmap is exhausted. */ - while (entry->matchResult == NULL || - (entry->matchResult->ntuples >= 0 && - entry->offset >= entry->matchResult->ntuples) || - entry->matchResult->blockno < advancePastBlk || + while ((!BlockNumberIsValid(entry->matchResult.blockno)) || + (entry->matchResult.ntuples >= 0 && + entry->offset >= entry->matchResult.ntuples) || + entry->matchResult.blockno < advancePastBlk || (ItemPointerIsLossyPage(&advancePast) && - entry->matchResult->blockno == advancePastBlk)) + entry->matchResult.blockno == advancePastBlk)) { - entry->matchResult = tbm_iterate(entry->matchIterator); + tbm_iterate(entry->matchIterator, &entry->matchResult); - if (entry->matchResult == NULL) + if (!BlockNumberIsValid(entry->matchResult.blockno)) { ItemPointerSetInvalid(&entry->curItem); tbm_end_iterate(entry->matchIterator); @@ -858,10 +872,10 @@ entryGetItem(GinState *ginstate, GinScanEntry entry, * We're now on the first page after advancePast which has any * items on it. If it's a lossy result, return that. */ - if (entry->matchResult->ntuples < 0) + if (entry->matchResult.ntuples < 0) { ItemPointerSetLossyPage(&entry->curItem, - entry->matchResult->blockno); + entry->matchResult.blockno); /* * We might as well fall out of the loop; we could not @@ -875,27 +889,27 @@ entryGetItem(GinState *ginstate, GinScanEntry entry, * Not a lossy page. Skip over any offsets <= advancePast, and * return that. */ - if (entry->matchResult->blockno == advancePastBlk) + if (entry->matchResult.blockno == advancePastBlk) { /* * First, do a quick check against the last offset on the * page. If that's > advancePast, so are all the other * offsets, so just go back to the top to get the next page. */ - if (entry->matchResult->offsets[entry->matchResult->ntuples - 1] <= advancePastOff) + if (entry->matchResult.offsets[entry->matchResult.ntuples - 1] <= advancePastOff) { - entry->offset = entry->matchResult->ntuples; + entry->offset = entry->matchResult.ntuples; continue; } /* Otherwise scan to find the first item > advancePast */ - while (entry->matchResult->offsets[entry->offset] <= advancePastOff) + while (entry->matchResult.offsets[entry->offset] <= advancePastOff) entry->offset++; } ItemPointerSet(&entry->curItem, - entry->matchResult->blockno, - entry->matchResult->offsets[entry->offset]); + entry->matchResult.blockno, + entry->matchResult.offsets[entry->offset]); entry->offset++; /* Done unless we need to reduce the result */ diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c index af24d38544..033d525339 100644 --- a/src/backend/access/gin/ginscan.c +++ b/src/backend/access/gin/ginscan.c @@ -106,7 +106,7 @@ ginFillScanEntry(GinScanOpaque so, OffsetNumber attnum, ItemPointerSetMin(&scanEntry->curItem); scanEntry->matchBitmap = NULL; scanEntry->matchIterator = NULL; - scanEntry->matchResult = NULL; + scanEntry->matchResult.blockno = InvalidBlockNumber; scanEntry->list = NULL; scanEntry->nlist = 0; scanEntry->offset = InvalidOffsetNumber; diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 81a7488007..f7e4d1094d 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2180,7 +2180,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, Buffer buffer; Snapshot snapshot; int ntup; - TBMIterateResult *tbmres; + TBMIterateResult tbmres; hscan->rs_cindex = 0; hscan->rs_ntuples = 0; @@ -2192,9 +2192,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, { CHECK_FOR_INTERRUPTS(); - tbmres = bhs_iterate(scan->rs_bhs_iterator); + bhs_iterate(scan->rs_bhs_iterator, &tbmres); - if (tbmres == NULL) + if (!BlockNumberIsValid(tbmres.blockno)) { /* no more entries in the bitmap */ Assert(hscan->rs_empty_tuples_pending == 0); @@ -2209,11 +2209,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, * isolation though, as we need to examine all invisible tuples * reachable by the index. */ - } while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks); + } while (!IsolationIsSerializable() && tbmres.blockno >= hscan->rs_nblocks); /* Got a valid block */ - *blockno = tbmres->blockno; - *recheck = tbmres->recheck; + *blockno = tbmres.blockno; + *recheck = tbmres.recheck; /* * We can skip fetching the heap page if we don't need any fields from the @@ -2221,19 +2221,19 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, * 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)) + !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(tbmres.ntuples >= 0); Assert(hscan->rs_empty_tuples_pending >= 0); - hscan->rs_empty_tuples_pending += tbmres->ntuples; + hscan->rs_empty_tuples_pending += tbmres.ntuples; return true; } - block = tbmres->blockno; + block = tbmres.blockno; /* * Acquire pin on the target heap page, trading in any pin we held before. @@ -2262,7 +2262,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, /* * We need two separate strategies for lossy and non-lossy cases. */ - if (tbmres->ntuples >= 0) + if (tbmres.ntuples >= 0) { /* * Bitmap is non-lossy, so we just look through the offsets listed in @@ -2271,9 +2271,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, */ int curslot; - for (curslot = 0; curslot < tbmres->ntuples; curslot++) + for (curslot = 0; curslot < tbmres.ntuples; curslot++) { - OffsetNumber offnum = tbmres->offsets[curslot]; + OffsetNumber offnum = tbmres.offsets[curslot]; ItemPointerData tid; HeapTupleData heapTuple; @@ -2323,7 +2323,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, Assert(ntup <= MaxHeapTuplesPerPage); hscan->rs_ntuples = ntup; - if (tbmres->ntuples < 0) + if (tbmres.ntuples < 0) (*lossy_pages)++; else (*exact_pages)++; diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 076e1ff674..78f79aafff 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -77,15 +77,16 @@ bhs_begin_iterate(TIDBitmap *tbm, dsa_pointer shared_area, dsa_area *personal_ar return result; } -TBMIterateResult * -bhs_iterate(BitmapHeapIterator *iterator) +void +bhs_iterate(BitmapHeapIterator *iterator, TBMIterateResult *result) { Assert(iterator); + Assert(result); if (iterator->serial) - return tbm_iterate(iterator->serial); + tbm_iterate(iterator->serial, result); else - return tbm_shared_iterate(iterator->parallel); + tbm_shared_iterate(iterator->parallel, result); } void @@ -348,7 +349,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) #ifdef USE_PREFETCH ParallelBitmapHeapState *pstate = node->pstate; BitmapHeapIterator *prefetch_iterator = node->pf_iterator; - TBMIterateResult *tbmpre; + TBMIterateResult tbmpre; if (pstate == NULL) { @@ -360,8 +361,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) else if (prefetch_iterator) { /* Do not let the prefetch iterator get behind the main one */ - tbmpre = bhs_iterate(prefetch_iterator); - node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; + bhs_iterate(prefetch_iterator, &tbmpre); + node->pfblockno = tbmpre.blockno; } return; } @@ -394,8 +395,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (prefetch_iterator) { - tbmpre = bhs_iterate(prefetch_iterator); - node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; + bhs_iterate(prefetch_iterator, &tbmpre); + node->pfblockno = tbmpre.blockno; } } } @@ -462,10 +463,12 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) { while (node->prefetch_pages < node->prefetch_target) { - TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator); + TBMIterateResult tbmpre; bool skip_fetch; - if (tbmpre == NULL) + bhs_iterate(prefetch_iterator, &tbmpre); + + if (!BlockNumberIsValid(tbmpre.blockno)) { /* No more pages to prefetch */ bhs_end_iterate(prefetch_iterator); @@ -473,7 +476,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) break; } node->prefetch_pages++; - node->pfblockno = tbmpre->blockno; + node->pfblockno = tbmpre.blockno; /* * If we expect not to have to actually read this heap page, @@ -482,13 +485,13 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * prefetch_pages?) */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - !tbmpre->recheck && + !tbmpre.recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, - tbmpre->blockno, + tbmpre.blockno, &node->pvmbuffer)); if (!skip_fetch) - PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno); + PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre.blockno); } } @@ -501,7 +504,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) { while (1) { - TBMIterateResult *tbmpre; + TBMIterateResult tbmpre; bool do_prefetch = false; bool skip_fetch; @@ -520,8 +523,8 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) if (!do_prefetch) return; - tbmpre = bhs_iterate(prefetch_iterator); - if (tbmpre == NULL) + bhs_iterate(prefetch_iterator, &tbmpre); + if (!BlockNumberIsValid(tbmpre.blockno)) { /* No more pages to prefetch */ bhs_end_iterate(prefetch_iterator); @@ -529,17 +532,17 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) break; } - node->pfblockno = tbmpre->blockno; + 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 && + !tbmpre.recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, - tbmpre->blockno, + tbmpre.blockno, &node->pvmbuffer)); if (!skip_fetch) - PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno); + PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre.blockno); } } } diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index 1dc4c99bf9..309a44bdb8 100644 --- a/src/backend/nodes/tidbitmap.c +++ b/src/backend/nodes/tidbitmap.c @@ -172,7 +172,6 @@ struct TBMIterator int spageptr; /* next spages index */ int schunkptr; /* next schunks index */ int schunkbit; /* next bit to check in current schunk */ - TBMIterateResult output; }; /* @@ -213,7 +212,6 @@ struct TBMSharedIterator PTEntryArray *ptbase; /* pagetable element array */ PTIterationArray *ptpages; /* sorted exact page index list */ PTIterationArray *ptchunks; /* sorted lossy page index list */ - TBMIterateResult output; }; /* Local function prototypes */ @@ -944,20 +942,21 @@ tbm_advance_schunkbit(PagetableEntry *chunk, int *schunkbitp) /* * tbm_iterate - scan through next page of a TIDBitmap * - * Returns a TBMIterateResult representing one page, or NULL if there are - * no more pages to scan. Pages are guaranteed to be delivered in numerical - * order. If result->ntuples < 0, then the bitmap is "lossy" and failed to - * remember the exact tuples to look at on this page --- the caller must - * examine all tuples on the page and check if they meet the intended - * condition. If result->recheck is true, only the indicated tuples need - * be examined, but the condition must be rechecked anyway. (For ease of - * testing, recheck is always set true when ntuples < 0.) + * Caller must pass in a TBMIterateResult to be filled. + * + * Pages are guaranteed to be delivered in numerical order. tbmres->blockno is + * set to InvalidBlockNumber when there are no more pages to scan. If + * tbmres->ntuples < 0, then the bitmap is "lossy" and failed to remember the + * exact tuples to look at on this page --- the caller must examine all tuples + * on the page and check if they meet the intended condition. If + * tbmres->recheck is true, only the indicated tuples need be examined, but the + * condition must be rechecked anyway. (For ease of testing, recheck is always + * set true when ntuples < 0.) */ -TBMIterateResult * -tbm_iterate(TBMIterator *iterator) +void +tbm_iterate(TBMIterator *iterator, TBMIterateResult *tbmres) { TIDBitmap *tbm = iterator->tbm; - TBMIterateResult *output = &(iterator->output); Assert(tbm->iterating == TBM_ITERATING_PRIVATE); @@ -985,6 +984,7 @@ tbm_iterate(TBMIterator *iterator) * If both chunk and per-page data remain, must output the numerically * earlier page. */ + Assert(tbmres); if (iterator->schunkptr < tbm->nchunks) { PagetableEntry *chunk = tbm->schunks[iterator->schunkptr]; @@ -995,11 +995,11 @@ tbm_iterate(TBMIterator *iterator) chunk_blockno < tbm->spages[iterator->spageptr]->blockno) { /* Return a lossy page indicator from the chunk */ - output->blockno = chunk_blockno; - output->ntuples = -1; - output->recheck = true; + tbmres->blockno = chunk_blockno; + tbmres->ntuples = -1; + tbmres->recheck = true; iterator->schunkbit++; - return output; + return; } } @@ -1015,16 +1015,17 @@ tbm_iterate(TBMIterator *iterator) page = tbm->spages[iterator->spageptr]; /* scan bitmap to extract individual offset numbers */ - ntuples = tbm_extract_page_tuple(page, output); - output->blockno = page->blockno; - output->ntuples = ntuples; - output->recheck = page->recheck; + ntuples = tbm_extract_page_tuple(page, tbmres); + tbmres->blockno = page->blockno; + tbmres->ntuples = ntuples; + tbmres->recheck = page->recheck; iterator->spageptr++; - return output; + return; } /* Nothing more in the bitmap */ - return NULL; + tbmres->blockno = InvalidBlockNumber; + return; } /* @@ -1034,10 +1035,9 @@ tbm_iterate(TBMIterator *iterator) * across multiple processes. We need to acquire the iterator LWLock, * before accessing the shared members. */ -TBMIterateResult * -tbm_shared_iterate(TBMSharedIterator *iterator) +void +tbm_shared_iterate(TBMSharedIterator *iterator, TBMIterateResult *tbmres) { - TBMIterateResult *output = &iterator->output; TBMSharedIteratorState *istate = iterator->state; PagetableEntry *ptbase = NULL; int *idxpages = NULL; @@ -1088,13 +1088,13 @@ tbm_shared_iterate(TBMSharedIterator *iterator) chunk_blockno < ptbase[idxpages[istate->spageptr]].blockno) { /* Return a lossy page indicator from the chunk */ - output->blockno = chunk_blockno; - output->ntuples = -1; - output->recheck = true; + tbmres->blockno = chunk_blockno; + tbmres->ntuples = -1; + tbmres->recheck = true; istate->schunkbit++; LWLockRelease(&istate->lock); - return output; + return; } } @@ -1104,21 +1104,22 @@ tbm_shared_iterate(TBMSharedIterator *iterator) int ntuples; /* scan bitmap to extract individual offset numbers */ - ntuples = tbm_extract_page_tuple(page, output); - output->blockno = page->blockno; - output->ntuples = ntuples; - output->recheck = page->recheck; + ntuples = tbm_extract_page_tuple(page, tbmres); + tbmres->blockno = page->blockno; + tbmres->ntuples = ntuples; + tbmres->recheck = page->recheck; istate->spageptr++; LWLockRelease(&istate->lock); - return output; + return; } LWLockRelease(&istate->lock); /* Nothing more in the bitmap */ - return NULL; + tbmres->blockno = InvalidBlockNumber; + return; } /* diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index 3013a44bae..3b432263bb 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -353,7 +353,7 @@ typedef struct GinScanEntryData /* for a partial-match or full-scan query, we accumulate all TIDs here */ TIDBitmap *matchBitmap; TBMIterator *matchIterator; - TBMIterateResult *matchResult; + TBMIterateResult matchResult; /* used for Posting list and one page in Posting tree */ ItemPointerData *list; diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h index cb56d20dc6..3c330f86e6 100644 --- a/src/include/executor/nodeBitmapHeapscan.h +++ b/src/include/executor/nodeBitmapHeapscan.h @@ -34,7 +34,7 @@ typedef struct BitmapHeapIterator struct TBMSharedIterator *parallel; } BitmapHeapIterator; -extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator); +extern void bhs_iterate(BitmapHeapIterator *iterator, TBMIterateResult *result); extern void bhs_end_iterate(BitmapHeapIterator *iterator); diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h index 432fae5296..f000c1af28 100644 --- a/src/include/nodes/tidbitmap.h +++ b/src/include/nodes/tidbitmap.h @@ -72,8 +72,8 @@ extern bool tbm_is_empty(const TIDBitmap *tbm); extern TBMIterator *tbm_begin_iterate(TIDBitmap *tbm); extern dsa_pointer tbm_prepare_shared_iterate(TIDBitmap *tbm); -extern TBMIterateResult *tbm_iterate(TBMIterator *iterator); -extern TBMIterateResult *tbm_shared_iterate(TBMSharedIterator *iterator); +extern void tbm_iterate(TBMIterator *iterator, TBMIterateResult *tbmres); +extern void tbm_shared_iterate(TBMSharedIterator *iterator, TBMIterateResult *tbmres); extern void tbm_end_iterate(TBMIterator *iterator); extern void tbm_end_shared_iterate(TBMSharedIterator *iterator); extern TBMSharedIterator *tbm_attach_shared_iterate(dsa_area *dsa, -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0014-Push-BitmapHeapScan-prefetch-code-into-heapam.c.patch" From 5eb00d2b400eb58e532a128946a57f79aef6434d Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 09:42:23 -0400 Subject: [PATCH v11 14/17] Push BitmapHeapScan prefetch code into heapam.c In preparation for transitioning to using the streaming read API for prefetching [1], move all of the BitmapHeapScanState members related to prefetching and the functions for accessing them into the HeapScanDescData and TableScanDescData. Members that still need to be accessed in BitmapHeapNext() could not be moved into heap AM-specific code. Specifically, parallel iterator setup requires several components which seem odd to pass to the table AM API. [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com --- src/backend/access/heap/heapam.c | 26 ++ src/backend/access/heap/heapam_handler.c | 262 +++++++++++++++++ src/backend/executor/nodeBitmapHeapscan.c | 341 ++-------------------- src/include/access/heapam.h | 17 ++ src/include/access/relscan.h | 8 + src/include/access/tableam.h | 26 +- src/include/nodes/execnodes.h | 14 - 7 files changed, 355 insertions(+), 339 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index ed3a3607b7..614d715fc7 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -948,8 +948,16 @@ 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_base.blockno = InvalidBlockNumber; + scan->rs_vmbuffer = InvalidBuffer; scan->rs_empty_tuples_pending = 0; + scan->pvmbuffer = InvalidBuffer; + + scan->pfblockno = InvalidBlockNumber; + scan->prefetch_target = -1; + scan->prefetch_pages = 0; /* * Disable page-at-a-time mode if it's not a MVCC-safe snapshot. @@ -1032,6 +1040,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE; } + scan->rs_base.blockno = InvalidBlockNumber; + + scan->pfblockno = InvalidBlockNumber; + scan->prefetch_target = -1; + scan->prefetch_pages = 0; + /* * unpin scan buffers */ @@ -1044,6 +1058,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, scan->rs_vmbuffer = InvalidBuffer; } + if (BufferIsValid(scan->pvmbuffer)) + { + ReleaseBuffer(scan->pvmbuffer); + scan->pvmbuffer = InvalidBuffer; + } + /* * reinitialize scan descriptor */ @@ -1069,6 +1089,12 @@ heap_endscan(TableScanDesc sscan) scan->rs_vmbuffer = InvalidBuffer; } + if (BufferIsValid(scan->pvmbuffer)) + { + ReleaseBuffer(scan->pvmbuffer); + scan->pvmbuffer = 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 f7e4d1094d..68bbb6f88c 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -61,6 +61,9 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer, OffsetNumber tupoffset); static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan); +static inline void BitmapAdjustPrefetchIterator(HeapScanDesc scan); +static inline void BitmapAdjustPrefetchTarget(HeapScanDesc scan); +static inline void BitmapPrefetch(HeapScanDesc scan); static const TableAmRoutine heapam_methods; @@ -2170,6 +2173,73 @@ 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(HeapScanDesc scan) +{ +#ifdef USE_PREFETCH + BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator; + ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; + 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) + { + /* Do not let the prefetch iterator get behind the main one */ + bhs_iterate(prefetch_iterator, &tbmpre); + scan->pfblockno = tbmpre.blockno; + } + return; + } + + /* + * Adjusting the prefetch iterator before invoking + * table_scan_bitmap_next_block() keeps prefetch distance higher across + * the parallel workers. + */ + if (scan->rs_base.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) + { + bhs_iterate(prefetch_iterator, &tbmpre); + scan->pfblockno = tbmpre.blockno; + } + } + } +#endif /* USE_PREFETCH */ +} + static bool heapam_scan_bitmap_next_block(TableScanDesc scan, bool *recheck, BlockNumber *blockno, @@ -2188,6 +2258,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, *blockno = InvalidBlockNumber; *recheck = true; + BitmapAdjustPrefetchIterator(hscan); + do { CHECK_FOR_INTERRUPTS(); @@ -2328,6 +2400,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->bm_parallel == NULL && + scan->rs_pf_bhs_iterator && + hscan->pfblockno > hscan->rs_base.blockno) + elog(ERROR, "prefetch and main iterators are out of sync"); + + /* Adjust the prefetch target */ + BitmapAdjustPrefetchTarget(hscan); + /* * 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, @@ -2338,6 +2422,154 @@ 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(HeapScanDesc scan) +{ +#ifdef USE_PREFETCH + ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; + int prefetch_maximum = scan->rs_base.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(HeapScanDesc scan) +{ +#ifdef USE_PREFETCH + ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; + BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator; + + if (pstate == NULL) + { + if (prefetch_iterator) + { + while (scan->prefetch_pages < scan->prefetch_target) + { + TBMIterateResult tbmpre; + bool skip_fetch; + + bhs_iterate(prefetch_iterator, &tbmpre); + + if (!BlockNumberIsValid(tbmpre.blockno)) + { + /* No more pages to prefetch */ + bhs_end_iterate(prefetch_iterator); + scan->rs_base.rs_pf_bhs_iterator = NULL; + 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->rs_base.rs_flags & SO_NEED_TUPLE) && + !tbmpre.recheck && + VM_ALL_VISIBLE(scan->rs_base.rs_rd, + tbmpre.blockno, + &scan->pvmbuffer)); + + if (!skip_fetch) + PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno); + } + } + + return; + } + + if (pstate->prefetch_pages < pstate->prefetch_target) + { + if (prefetch_iterator) + { + 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; + + bhs_iterate(prefetch_iterator, &tbmpre); + if (!BlockNumberIsValid(tbmpre.blockno)) + { + /* No more pages to prefetch */ + bhs_end_iterate(prefetch_iterator); + scan->rs_base.rs_pf_bhs_iterator = NULL; + break; + } + + scan->pfblockno = tbmpre.blockno; + + /* As above, skip prefetch if we expect not to need page */ + skip_fetch = (!(scan->rs_base.rs_flags & SO_NEED_TUPLE) && + !tbmpre.recheck && + VM_ALL_VISIBLE(scan->rs_base.rs_rd, + tbmpre.blockno, + &scan->pvmbuffer)); + + if (!skip_fetch) + PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno); + } + } + } +#endif /* USE_PREFETCH */ +} + static bool heapam_scan_bitmap_next_tuple(TableScanDesc scan, TupleTableSlot *slot) @@ -2363,6 +2595,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 (!scan->bm_parallel) + { + if (hscan->prefetch_target < scan->prefetch_maximum) + hscan->prefetch_target++; + } + else if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum) + { + /* take spinlock while updating shared state */ + SpinLockAcquire(&scan->bm_parallel->mutex); + if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum) + scan->bm_parallel->prefetch_target++; + SpinLockRelease(&scan->bm_parallel->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(hscan); +#endif /* USE_PREFETCH */ + targoffset = hscan->rs_vistuples[hscan->rs_cindex]; page = BufferGetPage(hscan->rs_cbuf); lp = PageGetItemId(page, targoffset); diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 78f79aafff..187b288e68 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); static BitmapHeapIterator *bhs_begin_iterate(TIDBitmap *tbm, dsa_pointer shared_area, @@ -122,7 +118,6 @@ BitmapHeapNext(BitmapHeapScanState *node) TableScanDesc scan; TIDBitmap *tbm; TupleTableSlot *slot; - ParallelBitmapHeapState *pstate = node->pstate; dsa_area *dsa = node->ss.ps.state->es_query_dsa; /* @@ -142,7 +137,7 @@ 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) @@ -154,7 +149,16 @@ BitmapHeapNext(BitmapHeapScanState *node) bool 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. + */ + int pf_maximum = 0; +#ifdef USE_PREFETCH + pf_maximum = get_tablespace_io_concurrency(node->ss.ss_currentRelation->rd_rel->reltablespace); +#endif + + if (!node->pstate || init_shared_state) { tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node)); @@ -169,23 +173,16 @@ 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(tbm); #ifdef USE_PREFETCH - if (node->prefetch_maximum > 0) + if (pf_maximum > 0) { - pstate->prefetch_iterator = + node->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; } #endif /* We have initialized the shared state so wake up others. */ - BitmapDoneInitializingSharedState(pstate); + BitmapDoneInitializingSharedState(node->pstate); } } @@ -216,19 +213,19 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } + scan->prefetch_maximum = pf_maximum; + scan->bm_parallel = node->pstate; + scan->rs_bhs_iterator = bhs_begin_iterate(tbm, - pstate ? pstate->tbmiterator : InvalidDsaPointer, + scan->bm_parallel ? scan->bm_parallel->tbmiterator : InvalidDsaPointer, dsa); #ifdef USE_PREFETCH - if (node->prefetch_maximum > 0) + if (scan->prefetch_maximum > 0) { - node->pf_iterator = bhs_begin_iterate(tbm, - pstate ? pstate->prefetch_iterator : InvalidDsaPointer, - dsa); - /* Only used for serial BHS */ - node->prefetch_pages = 0; - node->prefetch_target = -1; + scan->rs_pf_bhs_iterator = bhs_begin_iterate(tbm, + scan->bm_parallel ? scan->bm_parallel->prefetch_iterator : InvalidDsaPointer, + dsa); } #endif /* USE_PREFETCH */ @@ -243,36 +240,6 @@ BitmapHeapNext(BitmapHeapScanState *node) { 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 @@ -296,23 +263,9 @@ BitmapHeapNext(BitmapHeapScanState *node) new_page: - BitmapAdjustPrefetchIterator(node); - - if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno, + if (!table_scan_bitmap_next_block(scan, &node->recheck, &scan->blockno, &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 && - node->pfblockno > node->blockno) - elog(ERROR, "prefetch and main iterators are out of sync"); - - /* Adjust the prefetch target */ - BitmapAdjustPrefetchTarget(node); } /* @@ -336,219 +289,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) - { - /* Do not let the prefetch iterator get behind the main one */ - bhs_iterate(prefetch_iterator, &tbmpre); - node->pfblockno = tbmpre.blockno; - } - 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) - { - bhs_iterate(prefetch_iterator, &tbmpre); - node->pfblockno = tbmpre.blockno; - } - } - } -#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) - { - while (node->prefetch_pages < node->prefetch_target) - { - TBMIterateResult tbmpre; - bool skip_fetch; - - bhs_iterate(prefetch_iterator, &tbmpre); - - if (!BlockNumberIsValid(tbmpre.blockno)) - { - /* No more pages to prefetch */ - bhs_end_iterate(prefetch_iterator); - node->pf_iterator = NULL; - 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) - { - 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; - - bhs_iterate(prefetch_iterator, &tbmpre); - if (!BlockNumberIsValid(tbmpre.blockno)) - { - /* No more pages to prefetch */ - bhs_end_iterate(prefetch_iterator); - node->pf_iterator = NULL; - 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 */ @@ -594,22 +334,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) - { - bhs_end_iterate(node->pf_iterator); - node->pf_iterator = NULL; - } + /* 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 +378,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) table_endscan(scanDesc); /* - * release bitmaps and buffers if any + * release bitmaps if any */ - if (node->pf_iterator) - bhs_end_iterate(node->pf_iterator); if (node->tbm) tbm_free(node->tbm); - if (node->pvmbuffer != InvalidBuffer) - ReleaseBuffer(node->pvmbuffer); } /* ---------------------------------------------------------------- @@ -688,17 +414,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->pf_iterator = NULL; - 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 @@ -738,13 +458,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; /* @@ -828,7 +541,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 c7a538221a..4726d31403 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -82,6 +82,23 @@ typedef struct HeapScanDescData Buffer rs_vmbuffer; int rs_empty_tuples_pending; + /* + * These fields only used for prefetching in bitmap table scans + */ + + /* buffer for visibility-map lookups of prefetched pages */ + Buffer pvmbuffer; + + /* + * These fields only used in serial BHS + */ + /* 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; + /* 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/relscan.h b/src/include/access/relscan.h index fb22f305bf..7938b741d6 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -26,6 +26,7 @@ struct ParallelTableScanDescData; struct BitmapHeapIterator; +struct ParallelBitmapHeapState; /* * Generic descriptor for table scans. This is the base-class for table scans, @@ -45,6 +46,13 @@ typedef struct TableScanDescData /* Only used for Bitmap table scans */ struct BitmapHeapIterator *rs_bhs_iterator; + struct BitmapHeapIterator *rs_pf_bhs_iterator; + + /* maximum value for prefetch_target */ + int prefetch_maximum; + struct ParallelBitmapHeapState *bm_parallel; + /* used to validate BHS prefetch and current block stay in sync */ + BlockNumber blockno; /* * 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 bb2b79717c..799ac013d4 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -811,17 +811,6 @@ typedef struct TableAmRoutine * lossy_pages is incremented if the block's representation in the bitmap * is lossy, 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. */ @@ -971,6 +960,9 @@ table_beginscan_bm(Relation rel, Snapshot snapshot, result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); result->rs_bhs_iterator = NULL; + result->rs_pf_bhs_iterator = NULL; + result->prefetch_maximum = 0; + result->bm_parallel = NULL; return result; } @@ -1035,6 +1027,12 @@ table_endscan(TableScanDesc scan) { bhs_end_iterate(scan->rs_bhs_iterator); scan->rs_bhs_iterator = NULL; + + if (scan->rs_pf_bhs_iterator) + { + bhs_end_iterate(scan->rs_pf_bhs_iterator); + scan->rs_pf_bhs_iterator = NULL; + } } scan->rs_rd->rd_tableam->scan_end(scan); @@ -1051,6 +1049,12 @@ table_rescan(TableScanDesc scan, { bhs_end_iterate(scan->rs_bhs_iterator); scan->rs_bhs_iterator = NULL; + + if (scan->rs_pf_bhs_iterator) + { + bhs_end_iterate(scan->rs_pf_bhs_iterator); + scan->rs_pf_bhs_iterator = NULL; + } } scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 52cedd1b35..60916bf0d0 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1785,18 +1785,11 @@ struct BitmapHeapIterator; * * 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 @@ -1804,18 +1797,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; - struct BitmapHeapIterator *pf_iterator; ParallelBitmapHeapState *pstate; bool recheck; - BlockNumber blockno; - BlockNumber pfblockno; } BitmapHeapScanState; /* ---------------- -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0015-Remove-table_scan_bitmap_next_block.patch" From 7ac2a79503d7110c19a7c335fd5ab5422166b6a6 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 15:43:10 -0400 Subject: [PATCH v11 15/17] 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(). Heap AM's implementation of table_scan_bitmap_next_tuple() now calls the original heapam_scan_bitmap_next_block() function, but it is no longer an implementation of a table AM callback but instead a helper for heapam_scan_bitmap_next_tuple() --- 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 | 45 ++++++-------- src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 75 +++++------------------ 6 files changed, 63 insertions(+), 111 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 614d715fc7..5fc052f019 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -319,6 +319,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 68bbb6f88c..80f210b405 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2167,12 +2167,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 * @@ -2206,8 +2200,8 @@ BitmapAdjustPrefetchIterator(HeapScanDesc 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->rs_base.prefetch_maximum > 0) { @@ -2570,30 +2564,43 @@ BitmapPrefetch(HeapScanDesc 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) { HeapScanDesc hscan = (HeapScanDesc) scan; OffsetNumber targoffset; Page page; ItemId lp; - if (hscan->rs_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); - hscan->rs_empty_tuples_pending--; - return true; - } + 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 - */ - if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples) - return false; + if (!heapam_scan_bitmap_next_block(scan, recheck, &scan->blockno, + lossy_pages, exact_pages)) + return false; + } #ifdef USE_PREFETCH @@ -2975,7 +2982,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 ce637a5a5d..1d6b03d1ca 100644 --- a/src/backend/access/table/tableamapi.c +++ b/src/backend/access/table/tableamapi.c @@ -92,8 +92,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 187b288e68..2f9387e51a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -230,44 +230,35 @@ BitmapHeapNext(BitmapHeapScanState *node) #endif /* USE_PREFETCH */ node->initialized = true; - - goto new_page; } - 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, &scan->blockno, - &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 799ac013d4..5979ddee8b 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -799,36 +799,20 @@ 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. - * - * 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 block's representation in the bitmap - * is lossy, 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, - BlockNumber *blockno, - 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 @@ -2033,44 +2017,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 if bitmap is lossy for the selected block and exact_pages is - * incremented 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, - bool *recheck, BlockNumber *blockno, - 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, - blockno, 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 @@ -2081,7 +2034,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 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0016-v10-Read-Stream-API.patch" From 88dde25e1b9a9b340ae57161f098b6351d538f3c Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 26 Feb 2024 23:48:31 +1300 Subject: [PATCH v11 16/17] v10 Read Stream API Part 1: Provide vectored variant of ReadBuffer(). Break ReadBuffer() up into two steps: StartReadBuffers() and WaitReadBuffers(). This has two advantages: 1. Multiple consecutive blocks can be read with one system call. 2. Advice (hints of future reads) can optionally be issued to the kernel. The traditional ReadBuffer() function is now implemented in terms of those functions, to avoid duplication. For now we still only read a block at a time so there is no change to generated system calls yet, but later commits will provide infrastructure to help build up larger calls. Callers should respect the new GUC io_combine_limit, and the limit on per-backend pins which is now exposed as a public interface. With some more infrastructure in later work, StartReadBuffers() could be extended to start real asynchronous I/O instead of advice. Part 2: Provide API for streaming relation data. Introduce an abstraction where relation data can be accessed as a stream of buffers, with an implementation that is more efficient than the equivalent sequence of ReadBuffer() calls. Client code supplies a callback that can say which block number is wanted next, and then consumes individual buffers one at a time from the stream. This division allows read_stream.c to build up large calls to StartReadBuffers() up to io_combine_limit, and issue fadvise() advice ahead of time in a systematic way when random access is detected. This API is based on an idea from Andres Freund to pave the way for asynchronous I/O in future work as required to support direct I/O. The goal is to have an abstraction that insulates client code from future changes to the I/O subsystem. An extended API may be necessary in future for more complicated cases (for example recovery, whose LsnReadQueue device in xlogprefetcher.c is a distant cousin of this code that should eventually be replaced by it), but this basic API is sufficient for many common usage patterns involving predictable access to a single relation fork. Author: Thomas Munro Author: Andres Freund (optimization tweaks) Reviewed-by: Melanie Plageman Reviewed-by: Heikki Linnakangas Reviewed-by: Nazir Bilal Yavuz Reviewed-by: Dilip Kumar Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com --- doc/src/sgml/config.sgml | 14 + src/backend/storage/Makefile | 2 +- src/backend/storage/aio/Makefile | 14 + src/backend/storage/aio/meson.build | 5 + src/backend/storage/aio/read_stream.c | 733 ++++++++++++++++++ src/backend/storage/buffer/bufmgr.c | 709 +++++++++++------ src/backend/storage/buffer/localbuf.c | 14 +- src/backend/storage/meson.build | 1 + src/backend/utils/misc/guc_tables.c | 14 + src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/storage/bufmgr.h | 41 +- src/include/storage/read_stream.h | 62 ++ src/tools/pgindent/typedefs.list | 3 + 13 files changed, 1386 insertions(+), 227 deletions(-) create mode 100644 src/backend/storage/aio/Makefile create mode 100644 src/backend/storage/aio/meson.build create mode 100644 src/backend/storage/aio/read_stream.c create mode 100644 src/include/storage/read_stream.h diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 5468637e2e..f3736000ad 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2719,6 +2719,20 @@ include_dir 'conf.d' + + io_combine_limit (integer) + + io_combine_limit configuration parameter + + + + + Controls the largest I/O size in operations that combine I/O. + The default is 128kB. + + + + max_worker_processes (integer) diff --git a/src/backend/storage/Makefile b/src/backend/storage/Makefile index 8376cdfca2..eec03f6f2b 100644 --- a/src/backend/storage/Makefile +++ b/src/backend/storage/Makefile @@ -8,6 +8,6 @@ subdir = src/backend/storage top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -SUBDIRS = buffer file freespace ipc large_object lmgr page smgr sync +SUBDIRS = aio buffer file freespace ipc large_object lmgr page smgr sync include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/storage/aio/Makefile b/src/backend/storage/aio/Makefile new file mode 100644 index 0000000000..2f29a9ec4d --- /dev/null +++ b/src/backend/storage/aio/Makefile @@ -0,0 +1,14 @@ +# +# Makefile for storage/aio +# +# src/backend/storage/aio/Makefile +# + +subdir = src/backend/storage/aio +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global + +OBJS = \ + read_stream.o + +include $(top_srcdir)/src/backend/common.mk diff --git a/src/backend/storage/aio/meson.build b/src/backend/storage/aio/meson.build new file mode 100644 index 0000000000..10e1aa3b20 --- /dev/null +++ b/src/backend/storage/aio/meson.build @@ -0,0 +1,5 @@ +# Copyright (c) 2024, PostgreSQL Global Development Group + +backend_sources += files( + 'read_stream.c', +) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c new file mode 100644 index 0000000000..4e293e0df6 --- /dev/null +++ b/src/backend/storage/aio/read_stream.c @@ -0,0 +1,733 @@ +/*------------------------------------------------------------------------- + * + * read_stream.c + * Mechanism for accessing buffered relation data with look-ahead + * + * Code that needs to access relation data typically pins blocks one at a + * time, often in a predictable order that might be sequential or data-driven. + * Calling the simple ReadBuffer() function for each block is inefficient, + * because blocks that are not yet in the buffer pool require I/O operations + * that are small and might stall waiting for storage. This mechanism looks + * into the future and calls StartReadBuffers() and WaitReadBuffers() to read + * neighboring blocks together and ahead of time, with an adaptive look-ahead + * distance. + * + * A user-provided callback generates a stream of block numbers that is used + * to form reads of up to io_combine_limit, by attempting to merge them with a + * pending read. When that isn't possible, the existing pending read is sent + * to StartReadBuffers() so that a new one can begin to form. + * + * The algorithm for controlling the look-ahead distance tries to classify the + * stream into three ideal behaviors: + * + * A) No I/O is necessary, because the requested blocks are fully cached + * already. There is no benefit to looking ahead more than one block, so + * distance is 1. This is the default initial assumption. + * + * B) I/O is necessary, but fadvise is undesirable because the access is + * sequential, or impossible because direct I/O is enabled or the system + * doesn't support advice. There is no benefit in looking ahead more than + * io_combine_limit, because in this case only goal is larger read system + * calls. Looking further ahead would pin many buffers and perform + * speculative work looking ahead for no benefit. + * + * C) I/O is necesssary, it appears random, and this system supports fadvise. + * We'll look further ahead in order to reach the configured level of I/O + * concurrency. + * + * The distance increases rapidly and decays slowly, so that it moves towards + * those levels as different I/O patterns are discovered. For example, a + * sequential scan of fully cached data doesn't bother looking ahead, but a + * sequential scan that hits a region of uncached blocks will start issuing + * increasingly wide read calls until it plateaus at io_combine_limit. + * + * The main data structure is a circular queue of buffers of size + * max_pinned_buffers plus some extra space for technical reasons, ready to be + * returned by read_stream_next_buffer(). Each buffer also has an optional + * variable sized object that is passed from the callback to the consumer of + * buffers. + * + * Parallel to the queue of buffers, there is a circular queue of in-progress + * I/Os that have been started with StartReadBuffers(), and for which + * WaitReadBuffers() must be called before returning the buffer. + * + * For example, if the callback return block numbers 10, 42, 43, 60 in + * successive calls, then these data structures might appear as follows: + * + * buffers buf/data ios + * + * +----+ +-----+ +--------+ + * | | | | +----+ 42..44 | <- oldest_io_index + * +----+ +-----+ | +--------+ + * oldest_buffer_index -> | 10 | | ? | | +--+ 60..60 | + * +----+ +-----+ | | +--------+ + * | 42 | | ? |<-+ | | | <- next_io_index + * +----+ +-----+ | +--------+ + * | 43 | | ? | | | | + * +----+ +-----+ | +--------+ + * | 44 | | ? | | | | + * +----+ +-----+ | +--------+ + * | 60 | | ? |<---+ + * +----+ +-----+ + * next_buffer_index -> | | | | + * +----+ +-----+ + * + * In the example, 5 buffers are pinned, and the next buffer to be streamed to + * the client is block 10. Block 10 was a hit and has no associated I/O, but + * the range 42..44 requires an I/O wait before its buffers are returned, as + * does block 60. + * + * + * Portions Copyright (c) 2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * src/backend/storage/aio/read_stream.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "catalog/pg_tablespace.h" +#include "miscadmin.h" +#include "storage/fd.h" +#include "storage/smgr.h" +#include "storage/read_stream.h" +#include "utils/memdebug.h" +#include "utils/rel.h" +#include "utils/spccache.h" + +typedef struct InProgressIO +{ + int16 buffer_index; + ReadBuffersOperation op; +} InProgressIO; + +/* + * State for managing a stream of reads. + */ +struct ReadStream +{ + int16 max_ios; + int16 ios_in_progress; + int16 queue_size; + int16 max_pinned_buffers; + int16 pinned_buffers; + int16 distance; + bool advice_enabled; + + /* + * Sometimes we need to be able to 'unget' a block number to resolve a + * flow control problem when I/Os are split. + */ + BlockNumber unget_blocknum; + bool have_unget_blocknum; + + /* + * The callback that will tell us which block numbers to read, and an + * opaque pointer that will be pass to it for its own purposes. + */ + ReadStreamBlockNumberCB callback; + void *callback_private_data; + + /* Next expected block, for detecting sequential access. */ + BlockNumber seq_blocknum; + + /* The read operation we are currently preparing. */ + BlockNumber pending_read_blocknum; + int16 pending_read_nblocks; + + /* Space for buffers and optional per-buffer private data. */ + size_t per_buffer_data_size; + void *per_buffer_data; + + /* Read operations that have been started but not waited for yet. */ + InProgressIO *ios; + int16 oldest_io_index; + int16 next_io_index; + + /* Circular queue of buffers. */ + int16 oldest_buffer_index; /* Next pinned buffer to return */ + int16 next_buffer_index; /* Index of next buffer to pin */ + Buffer buffers[FLEXIBLE_ARRAY_MEMBER]; +}; + +/* + * Return a pointer to the per-buffer data by index. + */ +static inline void * +get_per_buffer_data(ReadStream *stream, int16 buffer_index) +{ + return (char *) stream->per_buffer_data + + stream->per_buffer_data_size * buffer_index; +} + +/* + * Ask the callback which block it would like us to read next, with a small + * buffer in front to allow streaming_unget_block() to work. + */ +static inline BlockNumber +read_stream_get_block(ReadStream *stream, void *per_buffer_data) +{ + if (!stream->have_unget_blocknum) + return stream->callback(stream, + stream->callback_private_data, + per_buffer_data); + + /* + * You can only unget one block, and next_buffer_index can't change across + * a get, unget, get sequence, so the callback's per_buffer_data, if any, + * is still present in the correct slot. We just have to return the + * previous block number. + */ + stream->have_unget_blocknum = false; + return stream->unget_blocknum; +} + +/* + * In order to deal with short reads in StartReadBuffers(), we sometimes need + * to defer handling of a block until later. + */ +static inline void +read_stream_unget_block(ReadStream *stream, BlockNumber blocknum) +{ + Assert(!stream->have_unget_blocknum); + stream->have_unget_blocknum = true; + stream->unget_blocknum = blocknum; +} + +static void +read_stream_start_pending_read(ReadStream *stream, bool suppress_advice) +{ + bool need_wait; + int nblocks; + int flags; + int16 io_index; + int16 overflow; + int16 buffer_index; + + /* This should only be called with a pending read. */ + Assert(stream->pending_read_nblocks > 0); + Assert(stream->pending_read_nblocks <= io_combine_limit); + + /* We had better not exceed the pin limit by starting this read. */ + Assert(stream->pinned_buffers + stream->pending_read_nblocks <= + stream->max_pinned_buffers); + + /* We had better not be overwriting an existing pinned buffer. */ + if (stream->pinned_buffers > 0) + Assert(stream->next_buffer_index != stream->oldest_buffer_index); + else + Assert(stream->next_buffer_index == stream->oldest_buffer_index); + + /* + * If advice hasn't been suppressed, this system supports it, and this + * isn't a strictly sequential pattern, then we'll issue advice. + */ + if (!suppress_advice && + stream->advice_enabled && + stream->pending_read_blocknum != stream->seq_blocknum) + flags = READ_BUFFERS_ISSUE_ADVICE; + else + flags = 0; + + /* We say how many blocks we want to read, but may be smaller on return. */ + buffer_index = stream->next_buffer_index; + io_index = stream->next_io_index; + nblocks = stream->pending_read_nblocks; + need_wait = StartReadBuffers(&stream->ios[io_index].op, + &stream->buffers[buffer_index], + stream->pending_read_blocknum, + &nblocks, + flags); + stream->pinned_buffers += nblocks; + + /* Remember whether we need to wait before returning this buffer. */ + if (!need_wait) + { + /* Look-ahead distance decays, no I/O necessary (behavior A). */ + if (stream->distance > 1) + stream->distance--; + } + else + { + /* + * Remember to call WaitReadBuffers() before returning head buffer. + * Look-ahead distance will be adjusted after waiting. + */ + stream->ios[io_index].buffer_index = buffer_index; + if (++stream->next_io_index == stream->max_ios) + stream->next_io_index = 0; + Assert(stream->ios_in_progress < stream->max_ios); + stream->ios_in_progress++; + stream->seq_blocknum = stream->pending_read_blocknum + nblocks; + } + + /* + * We gave a contiguous range of buffer space to StartReadBuffers(), but + * we want it to wrap around at queue_size. Slide overflowing buffers to + * the front of the array. + */ + overflow = (buffer_index + nblocks) - stream->queue_size; + if (overflow > 0) + memmove(&stream->buffers[0], + &stream->buffers[stream->queue_size], + sizeof(stream->buffers[0]) * overflow); + + /* Compute location of start of next read, without using % operator. */ + buffer_index += nblocks; + if (buffer_index >= stream->queue_size) + buffer_index -= stream->queue_size; + Assert(buffer_index >= 0 && buffer_index < stream->queue_size); + stream->next_buffer_index = buffer_index; + + /* Adjust the pending read to cover the remaining portion, if any. */ + stream->pending_read_blocknum += nblocks; + stream->pending_read_nblocks -= nblocks; +} + +static void +read_stream_look_ahead(ReadStream *stream, bool suppress_advice) +{ + while (stream->ios_in_progress < stream->max_ios && + stream->pinned_buffers + stream->pending_read_nblocks < stream->distance) + { + BlockNumber blocknum; + int16 buffer_index; + void *per_buffer_data; + + if (stream->pending_read_nblocks == io_combine_limit) + { + read_stream_start_pending_read(stream, suppress_advice); + suppress_advice = false; + continue; + } + + /* + * See which block the callback wants next in the stream. We need to + * compute the index of the Nth block of the pending read including + * wrap-around, but we don't want to use the expensive % operator. + */ + buffer_index = stream->next_buffer_index + stream->pending_read_nblocks; + if (buffer_index >= stream->queue_size) + buffer_index -= stream->queue_size; + Assert(buffer_index >= 0 && buffer_index < stream->queue_size); + per_buffer_data = get_per_buffer_data(stream, buffer_index); + blocknum = read_stream_get_block(stream, per_buffer_data); + if (blocknum == InvalidBlockNumber) + { + stream->distance = 0; + break; + } + + /* Can we merge it with the pending read? */ + if (stream->pending_read_nblocks > 0 && + stream->pending_read_blocknum + stream->pending_read_nblocks == blocknum) + { + stream->pending_read_nblocks++; + continue; + } + + /* We have to start the pending read before we can build another. */ + if (stream->pending_read_nblocks > 0) + { + read_stream_start_pending_read(stream, suppress_advice); + suppress_advice = false; + if (stream->ios_in_progress == stream->max_ios) + { + /* And we've hit the limit. Rewind, and stop here. */ + read_stream_unget_block(stream, blocknum); + return; + } + } + + /* This is the start of a new pending read. */ + stream->pending_read_blocknum = blocknum; + stream->pending_read_nblocks = 1; + } + + /* + * Normally we don't start the pending read just because we've hit a + * limit, preferring to give it another chance to grow to a larger size + * once more buffers have been consumed. However, in cases where that + * can't possibly happen, we might as well start the read immediately. + */ + if (stream->pending_read_nblocks > 0 && + (stream->distance == stream->pending_read_nblocks || + stream->distance == 0) && + stream->ios_in_progress < stream->max_ios) + read_stream_start_pending_read(stream, suppress_advice); +} + +/* + * Create a new streaming read object that can be used to perform the + * equivalent of a series of ReadBuffer() calls for one fork of one relation. + * Internally, it generates larger vectored reads where possible by looking + * ahead. The callback should return block numbers or InvalidBlockNumber to + * signal end-of-stream, and if per_buffer_data_size is non-zero, it may also + * write extra data for each block into the space provided to it. It will + * also receive callback_private_data for its own purposes. + */ +ReadStream * +read_stream_begin_relation(int flags, + BufferAccessStrategy strategy, + BufferManagerRelation bmr, + ForkNumber forknum, + ReadStreamBlockNumberCB callback, + void *callback_private_data, + size_t per_buffer_data_size) +{ + ReadStream *stream; + size_t size; + int16 queue_size; + int16 max_ios; + uint32 max_pinned_buffers; + Oid tablespace_id; + + /* Make sure our bmr's smgr and persistent are populated. */ + if (bmr.smgr == NULL) + { + bmr.smgr = RelationGetSmgr(bmr.rel); + bmr.relpersistence = bmr.rel->rd_rel->relpersistence; + } + + /* + * Decide how many I/Os we will allow to run at the same time. That + * currently means advice to the kernel to tell it that we will soon read. + * This number also affects how far we look ahead for opportunities to + * start more I/Os. + */ + tablespace_id = bmr.smgr->smgr_rlocator.locator.spcOid; + if (!OidIsValid(MyDatabaseId) || + (bmr.rel && IsCatalogRelation(bmr.rel)) || + IsCatalogRelationOid(bmr.smgr->smgr_rlocator.locator.relNumber)) + { + /* + * Avoid circularity while trying to look up tablespace settings or + * before spccache.c is ready. + */ + max_ios = effective_io_concurrency; + } + else if (flags & READ_STREAM_MAINTENANCE) + max_ios = get_tablespace_maintenance_io_concurrency(tablespace_id); + else + max_ios = get_tablespace_io_concurrency(tablespace_id); + max_ios = Min(max_ios, PG_INT16_MAX); + + /* + * Choose the maximum number of buffers we're prepared to pin. We try to + * pin fewer if we can, though. We clamp it to at least io_combine_limit + * so that we can have a chance to build up a full io_combine_limit sized + * read, even when max_ios is zero. Be careful not to allow int16 to + * overflow (even though that's not possible with the current GUC range + * limits), allowing also for the spare entry and the overflow space. + */ + max_pinned_buffers = Max(max_ios * 4, io_combine_limit); + max_pinned_buffers = Min(max_pinned_buffers, + PG_INT16_MAX - io_combine_limit - 1); + + /* Don't allow this backend to pin more than its share of buffers. */ + if (SmgrIsTemp(bmr.smgr)) + LimitAdditionalLocalPins(&max_pinned_buffers); + else + LimitAdditionalPins(&max_pinned_buffers); + Assert(max_pinned_buffers > 0); + + /* + * We need one extra entry for buffers and per-buffer data, because users + * of per-buffer data have access to the object until the next call to + * read_stream_next_buffer(), so we need a gap between the head and tail + * of the queue so that we don't clobber it. + */ + queue_size = max_pinned_buffers + 1; + + /* + * Allocate the object, the buffers, the ios and per_data_data space in + * one big chunk. Though we have queue_size buffers, we want to be able + * to assume that all the buffers for a single read are contiguous (i.e. + * don't wrap around halfway through), so we allow temporary overflows of + * up to the maximum possible read size by allocating an extra + * io_combine_limit - 1 elements. + */ + size = offsetof(ReadStream, buffers); + size += sizeof(Buffer) * (queue_size + io_combine_limit - 1); + size += sizeof(InProgressIO) * Max(1, max_ios); + size += per_buffer_data_size * queue_size; + size += MAXIMUM_ALIGNOF * 2; + stream = (ReadStream *) palloc(size); + memset(stream, 0, offsetof(ReadStream, buffers)); + stream->ios = (InProgressIO *) + MAXALIGN(&stream->buffers[queue_size + io_combine_limit - 1]); + if (per_buffer_data_size > 0) + stream->per_buffer_data = (void *) + MAXALIGN(&stream->ios[Max(1, max_ios)]); + +#ifdef USE_PREFETCH + + /* + * This system supports prefetching advice. We can use it as long as + * direct I/O isn't enabled, the caller hasn't promised sequential access + * (overriding our detection heuristics), and max_ios hasn't been set to + * zero. + */ + if ((io_direct_flags & IO_DIRECT_DATA) == 0 && + (flags & READ_STREAM_SEQUENTIAL) == 0 && + max_ios > 0) + stream->advice_enabled = true; +#endif + + /* + * For now, max_ios = 0 is interpreted as max_ios = 1 with advice disabled + * above. If we had real asynchronous I/O we might need a slightly + * different definition. + */ + if (max_ios == 0) + max_ios = 1; + + stream->max_ios = max_ios; + stream->per_buffer_data_size = per_buffer_data_size; + stream->max_pinned_buffers = max_pinned_buffers; + stream->queue_size = queue_size; + + if (!bmr.smgr) + { + bmr.smgr = RelationGetSmgr(bmr.rel); + bmr.relpersistence = bmr.rel->rd_rel->relpersistence; + } + stream->callback = callback; + stream->callback_private_data = callback_private_data; + + /* + * Skip the initial ramp-up phase if the caller says we're going to be + * reading the whole relation. This way we start out assuming we'll be + * doing full io_combine_limit sized reads (behavior B). + */ + if (flags & READ_STREAM_FULL) + stream->distance = Min(max_pinned_buffers, io_combine_limit); + else + stream->distance = 1; + + /* + * Since we always currently always access the same relation, we can + * initialize parts of the ReadBuffersOperation objects and leave them + * that way, to avoid wasting CPU cycles writing to them for each read. + */ + for (int i = 0; i < max_ios; ++i) + { + stream->ios[i].op.bmr = bmr; + stream->ios[i].op.forknum = forknum; + stream->ios[i].op.strategy = strategy; + } + + return stream; +} + +/* + * Pull one pinned buffer out of a stream created with + * read_stream_begin_buffered(). Each call returns successive blocks in the + * order specified by the callback. If per_buffer_data_size was set to a + * non-zero size, *per_buffer_data receives a pointer to the extra per-buffer + * data that the callback had a chance to populate, which remains valid until + * the next call to read_stream_next_buffer(). When the stream runs out of + * data, InvalidBuffer is returned. The caller may decide to end the stream + * early at any time by calling read_stream_end_buffered(). + */ +Buffer +read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) +{ + Buffer buffer; + int16 oldest_buffer_index; + + /* + * A fast path for all-cached scans (behavior A). This is the same as the + * usual algorithm, but it is specialized for no I/O and no per-buffer + * data, so we can skip the queue management code, stay in the same buffer + * slot and use singular StartReadBuffer(). + */ + if (likely(per_buffer_data == NULL && + stream->ios_in_progress == 0 && + stream->pinned_buffers == 1 && + stream->distance == 1)) + { + BlockNumber next_blocknum; + + /* + * We have a pinned buffer that we need to serve up, but we also want + * to probe the next one before we return, just in case we need to + * start an I/O. We can re-use the same buffer slot, and an arbitrary + * I/O slot since they're all free. + */ + oldest_buffer_index = stream->oldest_buffer_index; + Assert((oldest_buffer_index + 1) % stream->queue_size == + stream->next_buffer_index); + buffer = stream->buffers[oldest_buffer_index]; + Assert(buffer != InvalidBuffer); + Assert(stream->pending_read_nblocks <= 1); + if (unlikely(stream->pending_read_nblocks == 1)) + { + next_blocknum = stream->pending_read_blocknum; + stream->pending_read_nblocks = 0; + } + else + next_blocknum = read_stream_get_block(stream, NULL); + if (unlikely(next_blocknum == InvalidBlockNumber)) + { + /* End of stream. */ + stream->distance = 0; + stream->next_buffer_index = oldest_buffer_index; + /* Pin transferred to caller. */ + stream->pinned_buffers = 0; + return buffer; + } + /* Call the special single block version, which is marginally faster. */ + if (unlikely(StartReadBuffer(&stream->ios[0].op, + &stream->buffers[oldest_buffer_index], + next_blocknum, + stream->advice_enabled ? + READ_BUFFERS_ISSUE_ADVICE : 0))) + { + /* I/O needed. We'll take the general path next time. */ + stream->oldest_io_index = 0; + stream->next_io_index = stream->max_ios > 1 ? 1 : 0; + stream->ios_in_progress = 1; + stream->ios[0].buffer_index = oldest_buffer_index; + stream->seq_blocknum = next_blocknum + 1; + /* Increase look ahead distance (move towards behavior B/C). */ + stream->distance = Min(2, stream->max_pinned_buffers); + } + /* Pin transferred to caller, got another one, no net change. */ + Assert(stream->pinned_buffers == 1); + return buffer; + } + + if (stream->pinned_buffers == 0) + { + Assert(stream->oldest_buffer_index == stream->next_buffer_index); + + /* End of stream reached? */ + if (stream->distance == 0) + return InvalidBuffer; + + /* + * The usual order of operations is that we look ahead at the bottom + * of this function after potentially finishing an I/O and making + * space for more, but if we're just starting up we'll need to crank + * the handle to get started. + */ + read_stream_look_ahead(stream, true); + + /* End of stream reached? */ + if (stream->pinned_buffers == 0) + { + Assert(stream->distance == 0); + return InvalidBuffer; + } + } + + /* Grab the oldest pinned buffer and associated per-buffer data. */ + Assert(stream->pinned_buffers > 0); + oldest_buffer_index = stream->oldest_buffer_index; + Assert(oldest_buffer_index >= 0 && + oldest_buffer_index < stream->queue_size); + buffer = stream->buffers[oldest_buffer_index]; + if (per_buffer_data) + *per_buffer_data = get_per_buffer_data(stream, oldest_buffer_index); + + Assert(BufferIsValid(buffer)); + + /* Do we have to wait for an associated I/O first? */ + if (stream->ios_in_progress > 0 && + stream->ios[stream->oldest_io_index].buffer_index == oldest_buffer_index) + { + int16 io_index = stream->oldest_io_index; + int16 distance; + + /* Sanity check that we still agree on the buffers. */ + Assert(stream->ios[io_index].op.buffers == + &stream->buffers[oldest_buffer_index]); + + WaitReadBuffers(&stream->ios[io_index].op); + + Assert(stream->ios_in_progress > 0); + stream->ios_in_progress--; + if (++stream->oldest_io_index == stream->max_ios) + stream->oldest_io_index = 0; + + if (stream->ios[io_index].op.flags & READ_BUFFERS_ISSUE_ADVICE) + { + /* Distance ramps up fast (behavior C). */ + distance = stream->distance * 2; + distance = Min(distance, stream->max_pinned_buffers); + stream->distance = distance; + } + else + { + /* No advice; move towards io_combine_limit (behavior B). */ + if (stream->distance > io_combine_limit) + { + stream->distance--; + } + else + { + distance = stream->distance * 2; + distance = Min(distance, io_combine_limit); + distance = Min(distance, stream->max_pinned_buffers); + stream->distance = distance; + } + } + } + +#ifdef CLOBBER_FREED_MEMORY + /* Clobber old buffer and per-buffer data for debugging purposes. */ + stream->buffers[oldest_buffer_index] = InvalidBuffer; + + /* + * The caller will get access to the per-buffer data, until the next call. + * We wipe the one before, which is never occupied because queue_size + * allowed one extra element. This will hopefully trip up client code + * that is holding a dangling pointer to it. + */ + if (stream->per_buffer_data) + wipe_mem(get_per_buffer_data(stream, + oldest_buffer_index == 0 ? + stream->queue_size - 1 : + oldest_buffer_index - 1), + stream->per_buffer_data_size); +#endif + + /* Pin transferred to caller. */ + Assert(stream->pinned_buffers > 0); + stream->pinned_buffers--; + + /* Advance oldest buffer, with wrap-around. */ + stream->oldest_buffer_index++; + if (stream->oldest_buffer_index == stream->queue_size) + stream->oldest_buffer_index = 0; + + /* Prepare for the next call. */ + read_stream_look_ahead(stream, false); + + return buffer; +} + +/* + * Release stream resources. + */ +void +read_stream_end(ReadStream *stream) +{ + Buffer buffer; + + /* Stop looking ahead. */ + stream->distance = 0; + + /* Unpin anything that wasn't consumed. */ + while ((buffer = read_stream_next_buffer(stream, NULL)) != InvalidBuffer) + ReleaseBuffer(buffer); + + Assert(stream->pinned_buffers == 0); + Assert(stream->ios_in_progress == 0); + + /* Release memory. */ + pfree(stream); +} diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f0f8d4259c..577bcf6e5d 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -19,6 +19,11 @@ * and pin it so that no one can destroy it while this process * is using it. * + * StartReadBuffers() -- as above, but for multiple contiguous blocks in + * two steps. + * + * WaitReadBuffers() -- second step of StartReadBuffers(). + * * ReleaseBuffer() -- unpin a buffer * * MarkBufferDirty() -- mark a pinned buffer's contents as "dirty". @@ -160,6 +165,9 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER; int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER; int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER; +/* Limit on how many blocks should be handled in single I/O operations. */ +int io_combine_limit = DEFAULT_IO_COMBINE_LIMIT; + /* local state for LockBufferForCleanup */ static BufferDesc *PinCountWaitBuf = NULL; @@ -471,10 +479,9 @@ ForgetPrivateRefCountEntry(PrivateRefCountEntry *ref) ) -static Buffer ReadBuffer_common(SMgrRelation smgr, char relpersistence, +static Buffer ReadBuffer_common(BufferManagerRelation bmr, ForkNumber forkNum, BlockNumber blockNum, - ReadBufferMode mode, BufferAccessStrategy strategy, - bool *hit); + ReadBufferMode mode, BufferAccessStrategy strategy); static BlockNumber ExtendBufferedRelCommon(BufferManagerRelation bmr, ForkNumber fork, BufferAccessStrategy strategy, @@ -500,7 +507,7 @@ static uint32 WaitBufHdrUnlocked(BufferDesc *buf); static int SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context); static void WaitIO(BufferDesc *buf); -static bool StartBufferIO(BufferDesc *buf, bool forInput); +static bool StartBufferIO(BufferDesc *buf, bool forInput, bool nowait); static void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits, bool forget_owner); static void AbortBufferIO(Buffer buffer); @@ -781,7 +788,6 @@ Buffer ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy) { - bool hit; Buffer buf; /* @@ -794,15 +800,9 @@ ReadBufferExtended(Relation reln, ForkNumber forkNum, BlockNumber blockNum, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary tables of other sessions"))); - /* - * Read the buffer, and update pgstat counters to reflect a cache hit or - * miss. - */ - pgstat_count_buffer_read(reln); - buf = ReadBuffer_common(RelationGetSmgr(reln), reln->rd_rel->relpersistence, - forkNum, blockNum, mode, strategy, &hit); - if (hit) - pgstat_count_buffer_hit(reln); + buf = ReadBuffer_common(BMR_REL(reln), + forkNum, blockNum, mode, strategy); + return buf; } @@ -822,13 +822,12 @@ ReadBufferWithoutRelcache(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy, bool permanent) { - bool hit; - SMgrRelation smgr = smgropen(rlocator, INVALID_PROC_NUMBER); - return ReadBuffer_common(smgr, permanent ? RELPERSISTENCE_PERMANENT : - RELPERSISTENCE_UNLOGGED, forkNum, blockNum, - mode, strategy, &hit); + return ReadBuffer_common(BMR_SMGR(smgr, permanent ? RELPERSISTENCE_PERMANENT : + RELPERSISTENCE_UNLOGGED), + forkNum, blockNum, + mode, strategy); } /* @@ -994,35 +993,146 @@ ExtendBufferedRelTo(BufferManagerRelation bmr, */ if (buffer == InvalidBuffer) { - bool hit; - Assert(extended_by == 0); - buffer = ReadBuffer_common(bmr.smgr, bmr.relpersistence, - fork, extend_to - 1, mode, strategy, - &hit); + buffer = ReadBuffer_common(bmr, fork, extend_to - 1, mode, strategy); } return buffer; } /* - * ReadBuffer_common -- common logic for all ReadBuffer variants - * - * *hit is set to true if the request was satisfied from shared buffer cache. + * Zero a buffer and lock it, as part of the implementation of + * RBM_ZERO_AND_LOCK or RBM_ZERO_AND_CLEANUP_LOCK. The buffer must be already + * pinned. It does not have to be valid, but it is valid and locked on + * return. */ -static Buffer -ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, - BlockNumber blockNum, ReadBufferMode mode, - BufferAccessStrategy strategy, bool *hit) +static void +ZeroBuffer(Buffer buffer, ReadBufferMode mode) { BufferDesc *bufHdr; - Block bufBlock; - bool found; + uint32 buf_state; + + Assert(mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK); + + if (BufferIsLocal(buffer)) + bufHdr = GetLocalBufferDescriptor(-buffer - 1); + else + { + bufHdr = GetBufferDescriptor(buffer - 1); + if (mode == RBM_ZERO_AND_LOCK) + LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); + else + LockBufferForCleanup(buffer); + } + + memset(BufferGetPage(buffer), 0, BLCKSZ); + + if (BufferIsLocal(buffer)) + { + buf_state = pg_atomic_read_u32(&bufHdr->state); + buf_state |= BM_VALID; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + } + else + { + buf_state = LockBufHdr(bufHdr); + buf_state |= BM_VALID; + UnlockBufHdr(bufHdr, buf_state); + } +} + +/* + * Pin a buffer for a given block. *foundPtr is set to true if the block was + * already present, or false if more work is required to either read it in or + * zero it. + */ +static inline Buffer +PinBufferForBlock(BufferManagerRelation bmr, + ForkNumber forkNum, + BlockNumber blockNum, + BufferAccessStrategy strategy, + bool *foundPtr) +{ + BufferDesc *bufHdr; + bool isLocalBuf; IOContext io_context; IOObject io_object; - bool isLocalBuf = SmgrIsTemp(smgr); - *hit = false; + Assert(blockNum != P_NEW); + + Assert(bmr.smgr); + + isLocalBuf = bmr.relpersistence == RELPERSISTENCE_TEMP; + if (isLocalBuf) + { + io_context = IOCONTEXT_NORMAL; + io_object = IOOBJECT_TEMP_RELATION; + } + else + { + io_context = IOContextForStrategy(strategy); + io_object = IOOBJECT_RELATION; + } + + TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend); + + if (isLocalBuf) + { + bufHdr = LocalBufferAlloc(bmr.smgr, forkNum, blockNum, foundPtr); + if (*foundPtr) + pgBufferUsage.local_blks_hit++; + } + else + { + bufHdr = BufferAlloc(bmr.smgr, bmr.relpersistence, forkNum, blockNum, + strategy, foundPtr, io_context); + if (*foundPtr) + pgBufferUsage.shared_blks_hit++; + } + if (bmr.rel) + { + /* + * While pgBufferUsage's "read" counter isn't bumped unless we reach + * WaitReadBuffers() (so, not for hits, and not for buffers that are + * zeroed instead), the per-relation stats always count them. + */ + pgstat_count_buffer_read(bmr.rel); + if (*foundPtr) + pgstat_count_buffer_hit(bmr.rel); + } + if (*foundPtr) + { + VacuumPageHit++; + pgstat_count_io_op(io_object, io_context, IOOP_HIT); + if (VacuumCostActive) + VacuumCostBalance += VacuumCostPageHit; + + TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, + bmr.smgr->smgr_rlocator.locator.spcOid, + bmr.smgr->smgr_rlocator.locator.dbOid, + bmr.smgr->smgr_rlocator.locator.relNumber, + bmr.smgr->smgr_rlocator.backend, + true); + } + + return BufferDescriptorGetBuffer(bufHdr); +} + +/* + * ReadBuffer_common -- common logic for all ReadBuffer variants + */ +static inline Buffer +ReadBuffer_common(BufferManagerRelation bmr, ForkNumber forkNum, + BlockNumber blockNum, ReadBufferMode mode, + BufferAccessStrategy strategy) +{ + ReadBuffersOperation operation; + Buffer buffer; + int flags; /* * Backward compatibility path, most code should use ExtendBufferedRel() @@ -1041,181 +1151,359 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) flags |= EB_LOCK_FIRST; - return ExtendBufferedRel(BMR_SMGR(smgr, relpersistence), - forkNum, strategy, flags); + return ExtendBufferedRel(bmr, forkNum, strategy, flags); } - TRACE_POSTGRESQL_BUFFER_READ_START(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend); - - if (isLocalBuf) + if (unlikely(mode == RBM_ZERO_AND_CLEANUP_LOCK || + mode == RBM_ZERO_AND_LOCK)) { - /* - * We do not use a BufferAccessStrategy for I/O of temporary tables. - * However, in some cases, the "strategy" may not be NULL, so we can't - * rely on IOContextForStrategy() to set the right IOContext for us. - * This may happen in cases like CREATE TEMPORARY TABLE AS... - */ - io_context = IOCONTEXT_NORMAL; - io_object = IOOBJECT_TEMP_RELATION; - bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, &found); - if (found) - pgBufferUsage.local_blks_hit++; - else if (mode == RBM_NORMAL || mode == RBM_NORMAL_NO_LOG || - mode == RBM_ZERO_ON_ERROR) - pgBufferUsage.local_blks_read++; + bool found; + + if (bmr.smgr == NULL) + { + bmr.smgr = RelationGetSmgr(bmr.rel); + bmr.relpersistence = bmr.rel->rd_rel->relpersistence; + } + + buffer = PinBufferForBlock(bmr, forkNum, blockNum, strategy, &found); + ZeroBuffer(buffer, mode); + return buffer; } + + if (mode == RBM_ZERO_ON_ERROR) + flags = READ_BUFFERS_ZERO_ON_ERROR; else + flags = 0; + operation.bmr = bmr; + operation.forknum = forkNum; + operation.strategy = strategy; + if (StartReadBuffer(&operation, + &buffer, + blockNum, + flags)) + WaitReadBuffers(&operation); + + return buffer; +} + +/* + * Single block version of the StartReadBuffers(). This might save a few + * instructions when called from another translation unit, if the compiler + * inlines the code and specializes for nblocks == 1. + */ +bool +StartReadBuffer(ReadBuffersOperation *operation, + Buffer *buffer, + BlockNumber blocknum, + int flags) +{ + int nblocks = 1; + bool result; + + result = StartReadBuffers(operation, buffer, blocknum, &nblocks, flags); + Assert(nblocks == 1); /* single block can't be short */ + + return result; +} + +/* + * Begin reading a range of blocks beginning at blockNum and extending for + * *nblocks. On return, up to *nblocks pinned buffers holding those blocks + * are written into the buffers array, and *nblocks is updated to contain the + * actual number, which may be fewer than requested. Caller sets some of the + * members of operation; see struct definition. + * + * If false is returned, no I/O is necessary. If true is returned, one I/O + * has been started, and WaitReadBuffers() must be called with the same + * operation object before the buffers are accessed. Along with the operation + * object, the caller-supplied array of buffers must remain valid until + * WaitReadBuffers() is called. + * + * Currently the I/O is only started with optional operating system advice, + * and the real I/O happens in WaitReadBuffers(). In future work, true I/O + * could be initiated here. + */ +inline bool +StartReadBuffers(ReadBuffersOperation *operation, + Buffer *buffers, + BlockNumber blockNum, + int *nblocks, + int flags) +{ + int actual_nblocks = *nblocks; + int io_buffers_len = 0; + + Assert(*nblocks > 0); + Assert(*nblocks <= MAX_IO_COMBINE_LIMIT); + + if (!operation->bmr.smgr) { - /* - * lookup the buffer. IO_IN_PROGRESS is set if the requested block is - * not currently in memory. - */ - io_context = IOContextForStrategy(strategy); - io_object = IOOBJECT_RELATION; - bufHdr = BufferAlloc(smgr, relpersistence, forkNum, blockNum, - strategy, &found, io_context); - if (found) - pgBufferUsage.shared_blks_hit++; - else if (mode == RBM_NORMAL || mode == RBM_NORMAL_NO_LOG || - mode == RBM_ZERO_ON_ERROR) - pgBufferUsage.shared_blks_read++; + operation->bmr.smgr = RelationGetSmgr(operation->bmr.rel); + operation->bmr.relpersistence = operation->bmr.rel->rd_rel->relpersistence; } - /* At this point we do NOT hold any locks. */ - - /* if it was already in the buffer pool, we're done */ - if (found) + for (int i = 0; i < actual_nblocks; ++i) { - /* Just need to update stats before we exit */ - *hit = true; - VacuumPageHit++; - pgstat_count_io_op(io_object, io_context, IOOP_HIT); + bool found; - if (VacuumCostActive) - VacuumCostBalance += VacuumCostPageHit; + buffers[i] = PinBufferForBlock(operation->bmr, + operation->forknum, + blockNum + i, + operation->strategy, + &found); - TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend, - found); + if (found) + { + /* + * Terminate the read as soon as we get a hit. It could be a + * single buffer hit, or it could be a hit that follows a readable + * range. We don't want to create more than one readable range, + * so we stop here. + */ + actual_nblocks = i + 1; + break; + } + else + { + /* Extend the readable range to cover this block. */ + io_buffers_len++; + } + } + *nblocks = actual_nblocks; - /* - * In RBM_ZERO_AND_LOCK mode the caller expects the page to be locked - * on return. - */ - if (!isLocalBuf) + if (io_buffers_len > 0) + { + /* Populate information needed for I/O. */ + operation->buffers = buffers; + operation->blocknum = blockNum; + operation->flags = flags; + operation->nblocks = actual_nblocks; + operation->io_buffers_len = io_buffers_len; + + if (flags & READ_BUFFERS_ISSUE_ADVICE) { - if (mode == RBM_ZERO_AND_LOCK) - LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), - LW_EXCLUSIVE); - else if (mode == RBM_ZERO_AND_CLEANUP_LOCK) - LockBufferForCleanup(BufferDescriptorGetBuffer(bufHdr)); + /* + * In theory we should only do this if PinBufferForBlock() had to + * allocate new buffers above. That way, if two calls to + * StartReadBuffers() were made for the same blocks before + * WaitReadBuffers(), only the first would issue the advice. + * That'd be a better simulation of true asynchronous I/O, which + * would only start the I/O once, but isn't done here for + * simplicity. Note also that the following call might actually + * issue two advice calls if we cross a segment boundary; in a + * true asynchronous version we might choose to process only one + * real I/O at a time in that case. + */ + smgrprefetch(operation->bmr.smgr, + operation->forknum, + blockNum, + operation->io_buffers_len); } - return BufferDescriptorGetBuffer(bufHdr); + /* Indicate that WaitReadBuffers() should be called. */ + return true; + } + else + { + return false; } +} + +static inline bool +WaitReadBuffersCanStartIO(Buffer buffer, bool nowait) +{ + if (BufferIsLocal(buffer)) + { + BufferDesc *bufHdr = GetLocalBufferDescriptor(-buffer - 1); + + return (pg_atomic_read_u32(&bufHdr->state) & BM_VALID) == 0; + } + else + return StartBufferIO(GetBufferDescriptor(buffer - 1), true, nowait); +} + +void +WaitReadBuffers(ReadBuffersOperation *operation) +{ + Buffer *buffers; + int nblocks; + BlockNumber blocknum; + ForkNumber forknum; + bool isLocalBuf; + IOContext io_context; + IOObject io_object; /* - * if we have gotten to this point, we have allocated a buffer for the - * page but its contents are not yet valid. IO_IN_PROGRESS is set for it, - * if it's a shared buffer. + * Currently operations are only allowed to include a read of some range, + * with an optional extra buffer that is already pinned at the end. So + * nblocks can be at most one more than io_buffers_len. */ - Assert(!(pg_atomic_read_u32(&bufHdr->state) & BM_VALID)); /* spinlock not needed */ + Assert((operation->nblocks == operation->io_buffers_len) || + (operation->nblocks == operation->io_buffers_len + 1)); - bufBlock = isLocalBuf ? LocalBufHdrGetBlock(bufHdr) : BufHdrGetBlock(bufHdr); + /* Find the range of the physical read we need to perform. */ + nblocks = operation->io_buffers_len; + if (nblocks == 0) + return; /* nothing to do */ + + buffers = &operation->buffers[0]; + blocknum = operation->blocknum; + forknum = operation->forknum; + + isLocalBuf = operation->bmr.relpersistence == RELPERSISTENCE_TEMP; + if (isLocalBuf) + { + io_context = IOCONTEXT_NORMAL; + io_object = IOOBJECT_TEMP_RELATION; + } + else + { + io_context = IOContextForStrategy(operation->strategy); + io_object = IOOBJECT_RELATION; + } /* - * Read in the page, unless the caller intends to overwrite it and just - * wants us to allocate a buffer. + * We count all these blocks as read by this backend. This is traditional + * behavior, but might turn out to be not true if we find that someone + * else has beaten us and completed the read of some of these blocks. In + * that case the system globally double-counts, but we traditionally don't + * count this as a "hit", and we don't have a separate counter for "miss, + * but another backend completed the read". */ - if (mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) - MemSet((char *) bufBlock, 0, BLCKSZ); + if (isLocalBuf) + pgBufferUsage.local_blks_read += nblocks; else + pgBufferUsage.shared_blks_read += nblocks; + + for (int i = 0; i < nblocks; ++i) { - instr_time io_start = pgstat_prepare_io_time(track_io_timing); + int io_buffers_len; + Buffer io_buffers[MAX_IO_COMBINE_LIMIT]; + void *io_pages[MAX_IO_COMBINE_LIMIT]; + instr_time io_start; + BlockNumber io_first_block; + + /* + * Skip this block if someone else has already completed it. If an + * I/O is already in progress in another backend, this will wait for + * the outcome: either done, or something went wrong and we will + * retry. + */ + if (!WaitReadBuffersCanStartIO(buffers[i], false)) + { + /* + * Report this as a 'hit' for this backend, even though it must + * have started out as a miss in PinBufferForBlock(). + */ + TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, blocknum + i, + operation->bmr.smgr->smgr_rlocator.locator.spcOid, + operation->bmr.smgr->smgr_rlocator.locator.dbOid, + operation->bmr.smgr->smgr_rlocator.locator.relNumber, + operation->bmr.smgr->smgr_rlocator.backend, + true); + continue; + } + + /* We found a buffer that we need to read in. */ + io_buffers[0] = buffers[i]; + io_pages[0] = BufferGetBlock(buffers[i]); + io_first_block = blocknum + i; + io_buffers_len = 1; - smgrread(smgr, forkNum, blockNum, bufBlock); + /* + * How many neighboring-on-disk blocks can we can scatter-read into + * other buffers at the same time? In this case we don't wait if we + * see an I/O already in progress. We already hold BM_IO_IN_PROGRESS + * for the head block, so we should get on with that I/O as soon as + * possible. We'll come back to this block again, above. + */ + while ((i + 1) < nblocks && + WaitReadBuffersCanStartIO(buffers[i + 1], true)) + { + /* Must be consecutive block numbers. */ + Assert(BufferGetBlockNumber(buffers[i + 1]) == + BufferGetBlockNumber(buffers[i]) + 1); + + io_buffers[io_buffers_len] = buffers[++i]; + io_pages[io_buffers_len++] = BufferGetBlock(buffers[i]); + } - pgstat_count_io_op_time(io_object, io_context, - IOOP_READ, io_start, 1); + io_start = pgstat_prepare_io_time(track_io_timing); + smgrreadv(operation->bmr.smgr, forknum, io_first_block, io_pages, io_buffers_len); + pgstat_count_io_op_time(io_object, io_context, IOOP_READ, io_start, + io_buffers_len); - /* check for garbage data */ - if (!PageIsVerifiedExtended((Page) bufBlock, blockNum, - PIV_LOG_WARNING | PIV_REPORT_STAT)) + /* Verify each block we read, and terminate the I/O. */ + for (int j = 0; j < io_buffers_len; ++j) { - if (mode == RBM_ZERO_ON_ERROR || zero_damaged_pages) + BufferDesc *bufHdr; + Block bufBlock; + + if (isLocalBuf) { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("invalid page in block %u of relation %s; zeroing out page", - blockNum, - relpath(smgr->smgr_rlocator, forkNum)))); - MemSet((char *) bufBlock, 0, BLCKSZ); + bufHdr = GetLocalBufferDescriptor(-io_buffers[j] - 1); + bufBlock = LocalBufHdrGetBlock(bufHdr); } else - ereport(ERROR, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("invalid page in block %u of relation %s", - blockNum, - relpath(smgr->smgr_rlocator, forkNum)))); - } - } - - /* - * In RBM_ZERO_AND_LOCK / RBM_ZERO_AND_CLEANUP_LOCK mode, grab the buffer - * content lock before marking the page as valid, to make sure that no - * other backend sees the zeroed page before the caller has had a chance - * to initialize it. - * - * Since no-one else can be looking at the page contents yet, there is no - * difference between an exclusive lock and a cleanup-strength lock. (Note - * that we cannot use LockBuffer() or LockBufferForCleanup() here, because - * they assert that the buffer is already valid.) - */ - if ((mode == RBM_ZERO_AND_LOCK || mode == RBM_ZERO_AND_CLEANUP_LOCK) && - !isLocalBuf) - { - LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_EXCLUSIVE); - } + { + bufHdr = GetBufferDescriptor(io_buffers[j] - 1); + bufBlock = BufHdrGetBlock(bufHdr); + } - if (isLocalBuf) - { - /* Only need to adjust flags */ - uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); + /* check for garbage data */ + if (!PageIsVerifiedExtended((Page) bufBlock, io_first_block + j, + PIV_LOG_WARNING | PIV_REPORT_STAT)) + { + if ((operation->flags & READ_BUFFERS_ZERO_ON_ERROR) || zero_damaged_pages) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid page in block %u of relation %s; zeroing out page", + io_first_block + j, + relpath(operation->bmr.smgr->smgr_rlocator, forknum)))); + memset(bufBlock, 0, BLCKSZ); + } + else + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("invalid page in block %u of relation %s", + io_first_block + j, + relpath(operation->bmr.smgr->smgr_rlocator, forknum)))); + } - buf_state |= BM_VALID; - pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); - } - else - { - /* Set BM_VALID, terminate IO, and wake up any waiters */ - TerminateBufferIO(bufHdr, false, BM_VALID, true); - } + /* Terminate I/O and set BM_VALID. */ + if (isLocalBuf) + { + uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); - VacuumPageMiss++; - if (VacuumCostActive) - VacuumCostBalance += VacuumCostPageMiss; + buf_state |= BM_VALID; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + } + else + { + /* Set BM_VALID, terminate IO, and wake up any waiters */ + TerminateBufferIO(bufHdr, false, BM_VALID, true); + } - TRACE_POSTGRESQL_BUFFER_READ_DONE(forkNum, blockNum, - smgr->smgr_rlocator.locator.spcOid, - smgr->smgr_rlocator.locator.dbOid, - smgr->smgr_rlocator.locator.relNumber, - smgr->smgr_rlocator.backend, - found); + /* Report I/Os as completing individually. */ + TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, io_first_block + j, + operation->bmr.smgr->smgr_rlocator.locator.spcOid, + operation->bmr.smgr->smgr_rlocator.locator.dbOid, + operation->bmr.smgr->smgr_rlocator.locator.relNumber, + operation->bmr.smgr->smgr_rlocator.backend, + false); + } - return BufferDescriptorGetBuffer(bufHdr); + VacuumPageMiss += io_buffers_len; + if (VacuumCostActive) + VacuumCostBalance += VacuumCostPageMiss * io_buffers_len; + } } /* - * BufferAlloc -- subroutine for ReadBuffer. Handles lookup of a shared - * buffer. If no buffer exists already, selects a replacement - * victim and evicts the old page, but does NOT read in new page. + * BufferAlloc -- subroutine for PinBufferForBlock. Handles lookup of a shared + * buffer. If no buffer exists already, selects a replacement victim and + * evicts the old page, but does NOT read in new page. * * "strategy" can be a buffer replacement strategy object, or NULL for * the default strategy. The selected buffer's usage_count is advanced when @@ -1223,11 +1511,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, * * The returned buffer is pinned and is already marked as holding the * desired page. If it already did have the desired page, *foundPtr is - * set true. Otherwise, *foundPtr is set false and the buffer is marked - * as IO_IN_PROGRESS; ReadBuffer will now need to do I/O to fill it. - * - * *foundPtr is actually redundant with the buffer's BM_VALID flag, but - * we keep it for simplicity in ReadBuffer. + * set true. Otherwise, *foundPtr is set false. * * io_context is passed as an output parameter to avoid calling * IOContextForStrategy() when there is a shared buffers hit and no IO @@ -1286,19 +1570,10 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, { /* * We can only get here if (a) someone else is still reading in - * the page, or (b) a previous read attempt failed. We have to - * wait for any active read attempt to finish, and then set up our - * own read attempt if the page is still not BM_VALID. - * StartBufferIO does it all. + * the page, (b) a previous read attempt failed, or (c) someone + * called StartReadBuffers() but not yet WaitReadBuffers(). */ - if (StartBufferIO(buf, true)) - { - /* - * If we get here, previous attempts to read the buffer must - * have failed ... but we shall bravely try again. - */ - *foundPtr = false; - } + *foundPtr = false; } return buf; @@ -1363,19 +1638,10 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, { /* * We can only get here if (a) someone else is still reading in - * the page, or (b) a previous read attempt failed. We have to - * wait for any active read attempt to finish, and then set up our - * own read attempt if the page is still not BM_VALID. - * StartBufferIO does it all. + * the page, (b) a previous read attempt failed, or (c) someone + * called StartReadBuffers() but not yet WaitReadBuffers(). */ - if (StartBufferIO(existing_buf_hdr, true)) - { - /* - * If we get here, previous attempts to read the buffer must - * have failed ... but we shall bravely try again. - */ - *foundPtr = false; - } + *foundPtr = false; } return existing_buf_hdr; @@ -1407,15 +1673,9 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, LWLockRelease(newPartitionLock); /* - * Buffer contents are currently invalid. Try to obtain the right to - * start I/O. If StartBufferIO returns false, then someone else managed - * to read it before we did, so there's nothing left for BufferAlloc() to - * do. + * Buffer contents are currently invalid. */ - if (StartBufferIO(victim_buf_hdr, true)) - *foundPtr = false; - else - *foundPtr = true; + *foundPtr = false; return victim_buf_hdr; } @@ -1769,7 +2029,7 @@ again: * pessimistic, but outside of toy-sized shared_buffers it should allow * sufficient pins. */ -static void +void LimitAdditionalPins(uint32 *additional_pins) { uint32 max_backends; @@ -2034,7 +2294,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, buf_state &= ~BM_VALID; UnlockBufHdr(existing_hdr, buf_state); - } while (!StartBufferIO(existing_hdr, true)); + } while (!StartBufferIO(existing_hdr, true, false)); } else { @@ -2057,7 +2317,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, LWLockRelease(partition_lock); /* XXX: could combine the locked operations in it with the above */ - StartBufferIO(victim_buf_hdr, true); + StartBufferIO(victim_buf_hdr, true, false); } } @@ -2193,7 +2453,7 @@ MarkBufferDirty(Buffer buffer) uint32 old_buf_state; if (!BufferIsValid(buffer)) - elog(ERROR, "bad buffer ID: %d", buffer); + elog(PANIC, "bad buffer ID: %d", buffer); if (BufferIsLocal(buffer)) { @@ -2372,7 +2632,12 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy) else { /* - * If we previously pinned the buffer, it must surely be valid. + * If we previously pinned the buffer, it is likely to be valid, but + * it may not be if StartReadBuffers() was called and + * WaitReadBuffers() hasn't been called yet. We'll check by loading + * the flags without locking. This is racy, but it's OK to return + * false spuriously: when WaitReadBuffers() calls StartBufferIO(), + * it'll see that it's now valid. * * Note: We deliberately avoid a Valgrind client request here. * Individual access methods can optionally superimpose buffer page @@ -2381,7 +2646,7 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy) * that the buffer page is legitimately non-accessible here. We * cannot meddle with that. */ - result = true; + result = (pg_atomic_read_u32(&buf->state) & BM_VALID) != 0; } ref->refcount++; @@ -3449,7 +3714,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, * someone else flushed the buffer before we could, so we need not do * anything. */ - if (!StartBufferIO(buf, false)) + if (!StartBufferIO(buf, false, false)) return; /* Setup error traceback support for ereport() */ @@ -4560,7 +4825,7 @@ void ReleaseBuffer(Buffer buffer) { if (!BufferIsValid(buffer)) - elog(ERROR, "bad buffer ID: %d", buffer); + elog(PANIC, "bad buffer ID: %d", buffer); if (BufferIsLocal(buffer)) UnpinLocalBuffer(buffer); @@ -4627,7 +4892,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) Page page = BufferGetPage(buffer); if (!BufferIsValid(buffer)) - elog(ERROR, "bad buffer ID: %d", buffer); + elog(PANIC, "bad buffer ID: %d", buffer); if (BufferIsLocal(buffer)) { @@ -5184,9 +5449,15 @@ WaitIO(BufferDesc *buf) * * Returns true if we successfully marked the buffer as I/O busy, * false if someone else already did the work. + * + * If nowait is true, then we don't wait for an I/O to be finished by another + * backend. In that case, false indicates either that the I/O was already + * finished, or is still in progress. This is useful for callers that want to + * find out if they can perform the I/O as part of a larger operation, without + * waiting for the answer or distinguishing the reasons why not. */ static bool -StartBufferIO(BufferDesc *buf, bool forInput) +StartBufferIO(BufferDesc *buf, bool forInput, bool nowait) { uint32 buf_state; @@ -5199,6 +5470,8 @@ StartBufferIO(BufferDesc *buf, bool forInput) if (!(buf_state & BM_IO_IN_PROGRESS)) break; UnlockBufHdr(buf, buf_state); + if (nowait) + return false; WaitIO(buf); } @@ -5691,7 +5964,7 @@ ResOwnerReleaseBufferPin(Datum res) /* Like ReleaseBuffer, but don't call ResourceOwnerForgetBuffer */ if (!BufferIsValid(buffer)) - elog(ERROR, "bad buffer ID: %d", buffer); + elog(PANIC, "bad buffer ID: %d", buffer); if (BufferIsLocal(buffer)) UnpinLocalBufferNoOwner(buffer); diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index fcfac335a5..985a2c7049 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -108,10 +108,9 @@ PrefetchLocalBuffer(SMgrRelation smgr, ForkNumber forkNum, * LocalBufferAlloc - * Find or create a local buffer for the given page of the given relation. * - * API is similar to bufmgr.c's BufferAlloc, except that we do not need - * to do any locking since this is all local. Also, IO_IN_PROGRESS - * does not get set. Lastly, we support only default access strategy - * (hence, usage_count is always advanced). + * API is similar to bufmgr.c's BufferAlloc, except that we do not need to do + * any locking since this is all local. We support only default access + * strategy (hence, usage_count is always advanced). */ BufferDesc * LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, @@ -287,7 +286,7 @@ GetLocalVictimBuffer(void) } /* see LimitAdditionalPins() */ -static void +void LimitAdditionalLocalPins(uint32 *additional_pins) { uint32 max_pins; @@ -297,9 +296,10 @@ LimitAdditionalLocalPins(uint32 *additional_pins) /* * In contrast to LimitAdditionalPins() other backends don't play a role - * here. We can allow up to NLocBuffer pins in total. + * here. We can allow up to NLocBuffer pins in total, but it might not be + * initialized yet so read num_temp_buffers. */ - max_pins = (NLocBuffer - NLocalPinnedBuffers); + max_pins = (num_temp_buffers - NLocalPinnedBuffers); if (*additional_pins >= max_pins) *additional_pins = max_pins; diff --git a/src/backend/storage/meson.build b/src/backend/storage/meson.build index 40345bdca2..739d13293f 100644 --- a/src/backend/storage/meson.build +++ b/src/backend/storage/meson.build @@ -1,5 +1,6 @@ # Copyright (c) 2022-2024, PostgreSQL Global Development Group +subdir('aio') subdir('buffer') subdir('file') subdir('freespace') diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index abd9029451..313e393262 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -3112,6 +3112,20 @@ struct config_int ConfigureNamesInt[] = NULL }, + { + {"io_combine_limit", + PGC_USERSET, + RESOURCES_ASYNCHRONOUS, + gettext_noop("Limit on the size of data reads and writes."), + NULL, + GUC_UNIT_BLOCKS + }, + &io_combine_limit, + DEFAULT_IO_COMBINE_LIMIT, + 1, MAX_IO_COMBINE_LIMIT, + NULL, NULL, NULL + }, + { {"backend_flush_after", PGC_USERSET, RESOURCES_ASYNCHRONOUS, gettext_noop("Number of pages after which previously performed writes are flushed to disk."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 2244ee52f7..7fa6d5a64c 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -203,6 +203,7 @@ #backend_flush_after = 0 # measured in pages, 0 disables #effective_io_concurrency = 1 # 1-1000; 0 disables prefetching #maintenance_io_concurrency = 10 # 1-1000; 0 disables prefetching +#io_combine_limit = 128kB # usually 1-32 blocks (depends on OS) #max_worker_processes = 8 # (change requires restart) #max_parallel_workers_per_gather = 2 # limited by max_parallel_workers #max_parallel_maintenance_workers = 2 # limited by max_parallel_workers diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index d51d46d335..241f68c45e 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -14,6 +14,7 @@ #ifndef BUFMGR_H #define BUFMGR_H +#include "port/pg_iovec.h" #include "storage/block.h" #include "storage/buf.h" #include "storage/bufpage.h" @@ -133,6 +134,10 @@ extern PGDLLIMPORT bool track_io_timing; extern PGDLLIMPORT int effective_io_concurrency; extern PGDLLIMPORT int maintenance_io_concurrency; +#define MAX_IO_COMBINE_LIMIT PG_IOV_MAX +#define DEFAULT_IO_COMBINE_LIMIT Min(MAX_IO_COMBINE_LIMIT, (128 * 1024) / BLCKSZ) +extern PGDLLIMPORT int io_combine_limit; + extern PGDLLIMPORT int checkpoint_flush_after; extern PGDLLIMPORT int backend_flush_after; extern PGDLLIMPORT int bgwriter_flush_after; @@ -158,7 +163,6 @@ extern PGDLLIMPORT int32 *LocalRefCount; #define BUFFER_LOCK_SHARE 1 #define BUFFER_LOCK_EXCLUSIVE 2 - /* * prototypes for functions in bufmgr.c */ @@ -177,6 +181,38 @@ extern Buffer ReadBufferWithoutRelcache(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockNum, ReadBufferMode mode, BufferAccessStrategy strategy, bool permanent); + +#define READ_BUFFERS_ZERO_ON_ERROR 0x01 +#define READ_BUFFERS_ISSUE_ADVICE 0x02 + +struct ReadBuffersOperation +{ + /* The following members should be set by the caller. */ + BufferManagerRelation bmr; + ForkNumber forknum; + BufferAccessStrategy strategy; + + /* The following private members should not be accessed directly. */ + Buffer *buffers; + BlockNumber blocknum; + int flags; + int16 nblocks; + int16 io_buffers_len; +}; + +typedef struct ReadBuffersOperation ReadBuffersOperation; + +extern bool StartReadBuffer(ReadBuffersOperation *operation, + Buffer *buffer, + BlockNumber blocknum, + int flags); +extern bool StartReadBuffers(ReadBuffersOperation *operation, + Buffer *buffers, + BlockNumber blocknum, + int *nblocks, + int flags); +extern void WaitReadBuffers(ReadBuffersOperation *operation); + extern void ReleaseBuffer(Buffer buffer); extern void UnlockReleaseBuffer(Buffer buffer); extern bool BufferIsExclusiveLocked(Buffer buffer); @@ -250,6 +286,9 @@ extern bool HoldingBufferPinThatDelaysRecovery(void); extern bool BgBufferSync(struct WritebackContext *wb_context); +extern void LimitAdditionalPins(uint32 *additional_pins); +extern void LimitAdditionalLocalPins(uint32 *additional_pins); + /* in buf_init.c */ extern void InitBufferPool(void); extern Size BufferShmemSize(void); diff --git a/src/include/storage/read_stream.h b/src/include/storage/read_stream.h new file mode 100644 index 0000000000..9e5fa2acf1 --- /dev/null +++ b/src/include/storage/read_stream.h @@ -0,0 +1,62 @@ +/*------------------------------------------------------------------------- + * + * read_stream.h + * Mechanism for accessing buffered relation data with look-ahead + * + * + * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/storage/read_stream.h + * + *------------------------------------------------------------------------- + */ +#ifndef READ_STREAM_H +#define READ_STREAM_H + +#include "storage/bufmgr.h" + +/* Default tuning, reasonable for many users. */ +#define READ_STREAM_DEFAULT 0x00 + +/* + * I/O streams that are performing maintenance work on behalf of potentially + * many users, and thus should be governed by maintenance_io_concurrency + * instead of effective_io_concurrency. For example, VACUUM or CREATE INDEX. + */ +#define READ_STREAM_MAINTENANCE 0x01 + +/* + * We usually avoid issuing prefetch advice automatically when sequential + * access is detected, but this flag explicitly disables it, for cases that + * might not be correctly detected. Explicit advice is known to perform worse + * than letting the kernel (at least Linux) detect sequential access. + */ +#define READ_STREAM_SEQUENTIAL 0x02 + +/* + * We usually ramp up from smaller reads to larger ones, to support users who + * don't know if it's worth reading lots of buffers yet. This flag disables + * that, declaring ahead of time that we'll be reading all available buffers. + */ +#define READ_STREAM_FULL 0x04 + +struct ReadStream; +typedef struct ReadStream ReadStream; + +/* Callback that returns the next block number to read. */ +typedef BlockNumber (*ReadStreamBlockNumberCB) (ReadStream *stream, + void *callback_private_data, + void *per_buffer_data); + +extern ReadStream *read_stream_begin_relation(int flags, + BufferAccessStrategy strategy, + BufferManagerRelation bmr, + ForkNumber forknum, + ReadStreamBlockNumberCB callback, + void *callback_private_data, + size_t per_buffer_data_size); +extern Buffer read_stream_next_buffer(ReadStream *stream, void **per_buffer_private); +extern void read_stream_end(ReadStream *stream); + +#endif /* READ_STREAM_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a6562d19a6..16bfdf3444 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1215,6 +1215,7 @@ InjectionPointCacheEntry InjectionPointEntry InjectionPointSharedState InlineCodeBlock +InProgressIO InsertStmt Instrumentation Int128AggState @@ -2287,11 +2288,13 @@ ReInitializeDSMForeignScan_function ReScanForeignScan_function ReadBufPtrType ReadBufferMode +ReadBuffersOperation ReadBytePtrType ReadExtraTocPtrType ReadFunc ReadLocalXLogPageNoWaitPrivate ReadReplicationSlotCmd +ReadStream ReassignOwnedStmt RecheckForeignScan_function RecordCacheArrayEntry -- 2.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0017-BitmapHeapScan-uses-read-stream-API.patch" From bc1f127ff17a27f5bd9ac6dff96576707bbe1855 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 22 Mar 2024 16:51:40 -0400 Subject: [PATCH v11 17/17] BitmapHeapScan uses read stream API Remove all of the code to do prefetching from BitmapHeapScan code and rely on the read stream API prefetching. Heap table AM implements a read stream callback which uses the iterator to get the next valid block that needs to be fetched for the read stream API. ci-os-only: --- src/backend/access/heap/heapam.c | 96 ++++-- src/backend/access/heap/heapam_handler.c | 347 +++------------------- src/backend/executor/nodeBitmapHeapscan.c | 43 +-- src/include/access/heapam.h | 21 +- src/include/access/relscan.h | 6 - src/include/access/tableam.h | 14 - src/include/nodes/execnodes.h | 9 +- 7 files changed, 114 insertions(+), 422 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 5fc052f019..f7b1012b26 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -108,6 +108,8 @@ static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup); static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required, bool *copy); +static BlockNumber bitmapheap_stream_read_next(ReadStream *pgsr, void *pgsr_private, + void *per_buffer_data); /* * Each tuple lock mode has a corresponding heavyweight lock, and one or two @@ -330,6 +332,22 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock) if (key != NULL && scan->rs_base.rs_nkeys > 0) memcpy(scan->rs_base.rs_key, key, scan->rs_base.rs_nkeys * sizeof(ScanKeyData)); + if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN) + { + if (scan->rs_read_stream) + read_stream_end(scan->rs_read_stream); + + scan->rs_read_stream = read_stream_begin_relation(READ_STREAM_DEFAULT, + scan->rs_strategy, + BMR_REL(scan->rs_base.rs_rd), + MAIN_FORKNUM, + bitmapheap_stream_read_next, + scan, + sizeof(TBMIterateResult)); + + + } + /* * Currently, we only have a stats counter for sequential heap scans (but * e.g for bitmap scans the underlying bitmap index scans will be counted, @@ -950,16 +968,9 @@ 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_base.blockno = InvalidBlockNumber; - + scan->rs_read_stream = NULL; scan->rs_vmbuffer = InvalidBuffer; scan->rs_empty_tuples_pending = 0; - scan->pvmbuffer = InvalidBuffer; - - scan->pfblockno = InvalidBlockNumber; - scan->prefetch_target = -1; - scan->prefetch_pages = 0; /* * Disable page-at-a-time mode if it's not a MVCC-safe snapshot. @@ -1042,12 +1053,6 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE; } - scan->rs_base.blockno = InvalidBlockNumber; - - scan->pfblockno = InvalidBlockNumber; - scan->prefetch_target = -1; - scan->prefetch_pages = 0; - /* * unpin scan buffers */ @@ -1060,12 +1065,6 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params, scan->rs_vmbuffer = InvalidBuffer; } - if (BufferIsValid(scan->pvmbuffer)) - { - ReleaseBuffer(scan->pvmbuffer); - scan->pvmbuffer = InvalidBuffer; - } - /* * reinitialize scan descriptor */ @@ -1091,12 +1090,6 @@ heap_endscan(TableScanDesc sscan) scan->rs_vmbuffer = InvalidBuffer; } - if (BufferIsValid(scan->pvmbuffer)) - { - ReleaseBuffer(scan->pvmbuffer); - scan->pvmbuffer = InvalidBuffer; - } - /* * decrement relation reference count and free scan descriptor storage */ @@ -1114,6 +1107,9 @@ heap_endscan(TableScanDesc sscan) if (scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT) UnregisterSnapshot(scan->rs_base.rs_snapshot); + if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN && scan->rs_read_stream) + read_stream_end(scan->rs_read_stream); + pfree(scan); } @@ -10130,3 +10126,51 @@ HeapCheckForSerializableConflictOut(bool visible, Relation relation, CheckForSerializableConflictOut(relation, xid, snapshot); } + +static BlockNumber +bitmapheap_stream_read_next(ReadStream *pgsr, void *private_data, + void *per_buffer_data) +{ + TBMIterateResult *tbmres = per_buffer_data; + HeapScanDesc hdesc = (HeapScanDesc) private_data; + + for (;;) + { + CHECK_FOR_INTERRUPTS(); + + bhs_iterate(hdesc->rs_base.rs_bhs_iterator, tbmres); + + /* no more entries in the bitmap */ + if (!BlockNumberIsValid(tbmres->blockno)) + return InvalidBlockNumber; + + /* + * 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() && tbmres->blockno >= hdesc->rs_nblocks) + continue; + + /* + * 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 (!(hdesc->rs_base.rs_flags & SO_NEED_TUPLE) && + !tbmres->recheck && + VM_ALL_VISIBLE(hdesc->rs_base.rs_rd, tbmres->blockno, &hdesc->rs_vmbuffer)) + { + hdesc->rs_empty_tuples_pending += tbmres->ntuples; + continue; + } + + return tbmres->blockno; + } + + /* not reachable */ + Assert(false); +} diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 80f210b405..2e17a85d00 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -61,9 +61,6 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer, OffsetNumber tupoffset); static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan); -static inline void BitmapAdjustPrefetchIterator(HeapScanDesc scan); -static inline void BitmapAdjustPrefetchTarget(HeapScanDesc scan); -static inline void BitmapPrefetch(HeapScanDesc scan); static const TableAmRoutine heapam_methods; @@ -2167,147 +2164,68 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths, HEAP_USABLE_BYTES_PER_PAGE); } -/* - * 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(HeapScanDesc scan) -{ -#ifdef USE_PREFETCH - BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator; - ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; - 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) - { - /* Do not let the prefetch iterator get behind the main one */ - bhs_iterate(prefetch_iterator, &tbmpre); - scan->pfblockno = tbmpre.blockno; - } - return; - } - - /* - * Adjusting the prefetch iterator before invoking - * heapam_bitmap_next_block() keeps prefetch distance higher across the - * parallel workers. - */ - if (scan->rs_base.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) - { - bhs_iterate(prefetch_iterator, &tbmpre); - scan->pfblockno = tbmpre.blockno; - } - } - } -#endif /* USE_PREFETCH */ -} static bool -heapam_scan_bitmap_next_block(TableScanDesc scan, - bool *recheck, BlockNumber *blockno, +heapam_scan_bitmap_next_block(TableScanDesc scan, bool *recheck, long *lossy_pages, long *exact_pages) { HeapScanDesc hscan = (HeapScanDesc) scan; + void *per_buffer_data; BlockNumber block; Buffer buffer; Snapshot snapshot; int ntup; - TBMIterateResult tbmres; + TBMIterateResult *tbmres; + + Assert(hscan->rs_read_stream); hscan->rs_cindex = 0; hscan->rs_ntuples = 0; - *blockno = InvalidBlockNumber; *recheck = true; - BitmapAdjustPrefetchIterator(hscan); - - do + /* Release buffer containing previous block. */ + if (BufferIsValid(hscan->rs_cbuf)) { - CHECK_FOR_INTERRUPTS(); + ReleaseBuffer(hscan->rs_cbuf); + hscan->rs_cbuf = InvalidBuffer; + } - bhs_iterate(scan->rs_bhs_iterator, &tbmres); + hscan->rs_cbuf = read_stream_next_buffer(hscan->rs_read_stream, &per_buffer_data); - if (!BlockNumberIsValid(tbmres.blockno)) + if (BufferIsInvalid(hscan->rs_cbuf)) + { + if (BufferIsValid(hscan->rs_vmbuffer)) { - /* no more entries in the bitmap */ - Assert(hscan->rs_empty_tuples_pending == 0); - return false; + ReleaseBuffer(hscan->rs_vmbuffer); + hscan->rs_vmbuffer = InvalidBuffer; } /* - * 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. + * Bitmap is exhausted. Time to emit empty tuples if relevant. We emit + * all empty tuples at the end instead of emitting them per block we + * skip fetching. This is necessary because the streaming read API + * will only return TBMIterateResults for blocks actually fetched. + * When we skip fetching a block, we keep track of how many empty + * tuples to emit at the end of the BitmapHeapScan. We do not recheck + * all NULL tuples. */ - } while (!IsolationIsSerializable() && tbmres.blockno >= hscan->rs_nblocks); + *recheck = false; + return hscan->rs_empty_tuples_pending > 0; + } - /* Got a valid block */ - *blockno = tbmres.blockno; - *recheck = tbmres.recheck; + Assert(per_buffer_data); - /* - * 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); + tbmres = per_buffer_data; - hscan->rs_empty_tuples_pending += tbmres.ntuples; + Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno); - return true; - } + *recheck = tbmres->recheck; - block = tbmres.blockno; + hscan->rs_cblock = tbmres->blockno; + hscan->rs_ntuples = tbmres->ntuples; - /* - * Acquire pin on the target heap page, trading in any pin we held before. - */ - hscan->rs_cbuf = ReleaseAndReadBuffer(hscan->rs_cbuf, - scan->rs_rd, - block); - hscan->rs_cblock = block; + block = tbmres->blockno; buffer = hscan->rs_cbuf; snapshot = scan->rs_snapshot; @@ -2328,7 +2246,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, /* * We need two separate strategies for lossy and non-lossy cases. */ - if (tbmres.ntuples >= 0) + if (tbmres->ntuples >= 0) { /* * Bitmap is non-lossy, so we just look through the offsets listed in @@ -2337,9 +2255,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, */ int curslot; - for (curslot = 0; curslot < tbmres.ntuples; curslot++) + for (curslot = 0; curslot < tbmres->ntuples; curslot++) { - OffsetNumber offnum = tbmres.offsets[curslot]; + OffsetNumber offnum = tbmres->offsets[curslot]; ItemPointerData tid; HeapTupleData heapTuple; @@ -2389,23 +2307,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, Assert(ntup <= MaxHeapTuplesPerPage); hscan->rs_ntuples = ntup; - if (tbmres.ntuples < 0) + if (tbmres->ntuples < 0) (*lossy_pages)++; else (*exact_pages)++; - /* - * If serial, we can error out if the the prefetch block doesn't stay - * ahead of the current block. - */ - 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"); - - /* Adjust the prefetch target */ - BitmapAdjustPrefetchTarget(hscan); - /* * 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, @@ -2416,153 +2322,6 @@ 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(HeapScanDesc scan) -{ -#ifdef USE_PREFETCH - ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; - int prefetch_maximum = scan->rs_base.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(HeapScanDesc scan) -{ -#ifdef USE_PREFETCH - ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel; - BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator; - - if (pstate == NULL) - { - if (prefetch_iterator) - { - while (scan->prefetch_pages < scan->prefetch_target) - { - TBMIterateResult tbmpre; - bool skip_fetch; - - bhs_iterate(prefetch_iterator, &tbmpre); - - if (!BlockNumberIsValid(tbmpre.blockno)) - { - /* No more pages to prefetch */ - bhs_end_iterate(prefetch_iterator); - scan->rs_base.rs_pf_bhs_iterator = NULL; - 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->rs_base.rs_flags & SO_NEED_TUPLE) && - !tbmpre.recheck && - VM_ALL_VISIBLE(scan->rs_base.rs_rd, - tbmpre.blockno, - &scan->pvmbuffer)); - - if (!skip_fetch) - PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno); - } - } - - return; - } - - if (pstate->prefetch_pages < pstate->prefetch_target) - { - if (prefetch_iterator) - { - 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; - - bhs_iterate(prefetch_iterator, &tbmpre); - if (!BlockNumberIsValid(tbmpre.blockno)) - { - /* No more pages to prefetch */ - bhs_end_iterate(prefetch_iterator); - scan->rs_base.rs_pf_bhs_iterator = NULL; - break; - } - - scan->pfblockno = tbmpre.blockno; - - /* As above, skip prefetch if we expect not to need page */ - skip_fetch = (!(scan->rs_base.rs_flags & SO_NEED_TUPLE) && - !tbmpre.recheck && - VM_ALL_VISIBLE(scan->rs_base.rs_rd, - tbmpre.blockno, - &scan->pvmbuffer)); - - if (!skip_fetch) - PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno); - } - } - } -#endif /* USE_PREFETCH */ -} /* ------------------------------------------------------------------------ * Executor related callbacks for the heap AM @@ -2597,41 +2356,11 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan, return true; } - if (!heapam_scan_bitmap_next_block(scan, recheck, &scan->blockno, + if (!heapam_scan_bitmap_next_block(scan, recheck, lossy_pages, exact_pages)) 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 (!scan->bm_parallel) - { - if (hscan->prefetch_target < scan->prefetch_maximum) - hscan->prefetch_target++; - } - else if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum) - { - /* take spinlock while updating shared state */ - SpinLockAcquire(&scan->bm_parallel->mutex); - if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum) - scan->bm_parallel->prefetch_target++; - SpinLockRelease(&scan->bm_parallel->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(hscan); -#endif /* USE_PREFETCH */ - targoffset = hscan->rs_vistuples[hscan->rs_cindex]; page = BufferGetPage(hscan->rs_cbuf); lp = PageGetItemId(page, targoffset); diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2f9387e51a..f2662ea542 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -131,14 +131,6 @@ 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. */ if (!node->initialized) { @@ -149,15 +141,6 @@ BitmapHeapNext(BitmapHeapScanState *node) bool 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. - */ - int pf_maximum = 0; -#ifdef USE_PREFETCH - pf_maximum = get_tablespace_io_concurrency(node->ss.ss_currentRelation->rd_rel->reltablespace); -#endif - if (!node->pstate || init_shared_state) { tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node)); @@ -174,13 +157,7 @@ BitmapHeapNext(BitmapHeapScanState *node) * multiple processes to iterate jointly. */ node->pstate->tbmiterator = tbm_prepare_shared_iterate(tbm); -#ifdef USE_PREFETCH - if (pf_maximum > 0) - { - node->pstate->prefetch_iterator = - tbm_prepare_shared_iterate(tbm); - } -#endif + /* We have initialized the shared state so wake up others. */ BitmapDoneInitializingSharedState(node->pstate); } @@ -213,22 +190,12 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } - scan->prefetch_maximum = pf_maximum; scan->bm_parallel = node->pstate; scan->rs_bhs_iterator = bhs_begin_iterate(tbm, scan->bm_parallel ? scan->bm_parallel->tbmiterator : InvalidDsaPointer, dsa); -#ifdef USE_PREFETCH - if (scan->prefetch_maximum > 0) - { - scan->rs_pf_bhs_iterator = bhs_begin_iterate(tbm, - scan->bm_parallel ? scan->bm_parallel->prefetch_iterator : InvalidDsaPointer, - dsa); - } -#endif /* USE_PREFETCH */ - node->initialized = true; } @@ -525,14 +492,10 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node, return; pstate = shm_toc_allocate(pcxt->toc, sizeof(ParallelBitmapHeapState)); - pstate->tbmiterator = 0; - pstate->prefetch_iterator = 0; /* Initialize the mutex */ SpinLockInit(&pstate->mutex); - pstate->prefetch_pages = 0; - pstate->prefetch_target = -1; pstate->state = BM_INITIAL; ConditionVariableInit(&pstate->cv); @@ -563,11 +526,7 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node, if (DsaPointerIsValid(pstate->tbmiterator)) tbm_free_shared_area(dsa, pstate->tbmiterator); - if (DsaPointerIsValid(pstate->prefetch_iterator)) - tbm_free_shared_area(dsa, pstate->prefetch_iterator); - pstate->tbmiterator = InvalidDsaPointer; - pstate->prefetch_iterator = InvalidDsaPointer; } /* ---------------------------------------------------------------- diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 4726d31403..bb701cca08 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -26,6 +26,7 @@ #include "storage/dsm.h" #include "storage/lockdefs.h" #include "storage/shm_toc.h" +#include "storage/read_stream.h" #include "utils/relcache.h" #include "utils/snapshot.h" @@ -72,6 +73,9 @@ typedef struct HeapScanDescData */ ParallelBlockTableScanWorkerData *rs_parallelworkerdata; + /* Streaming read control object for scans supporting it */ + ReadStream *rs_read_stream; + /* * These fields are only used for bitmap scans for the "skip fetch" * optimization. Bitmap scans needing no fields from the heap may skip @@ -82,23 +86,6 @@ typedef struct HeapScanDescData Buffer rs_vmbuffer; int rs_empty_tuples_pending; - /* - * These fields only used for prefetching in bitmap table scans - */ - - /* buffer for visibility-map lookups of prefetched pages */ - Buffer pvmbuffer; - - /* - * These fields only used in serial BHS - */ - /* 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; - /* 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/relscan.h b/src/include/access/relscan.h index 7938b741d6..02893bf99b 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -46,13 +46,7 @@ typedef struct TableScanDescData /* Only used for Bitmap table scans */ struct BitmapHeapIterator *rs_bhs_iterator; - struct BitmapHeapIterator *rs_pf_bhs_iterator; - - /* maximum value for prefetch_target */ - int prefetch_maximum; struct ParallelBitmapHeapState *bm_parallel; - /* used to validate BHS prefetch and current block stay in sync */ - BlockNumber blockno; /* * 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 5979ddee8b..2b24d7441a 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -944,8 +944,6 @@ table_beginscan_bm(Relation rel, Snapshot snapshot, result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); result->rs_bhs_iterator = NULL; - result->rs_pf_bhs_iterator = NULL; - result->prefetch_maximum = 0; result->bm_parallel = NULL; return result; } @@ -1011,12 +1009,6 @@ table_endscan(TableScanDesc scan) { bhs_end_iterate(scan->rs_bhs_iterator); scan->rs_bhs_iterator = NULL; - - if (scan->rs_pf_bhs_iterator) - { - bhs_end_iterate(scan->rs_pf_bhs_iterator); - scan->rs_pf_bhs_iterator = NULL; - } } scan->rs_rd->rd_tableam->scan_end(scan); @@ -1033,12 +1025,6 @@ table_rescan(TableScanDesc scan, { bhs_end_iterate(scan->rs_bhs_iterator); scan->rs_bhs_iterator = NULL; - - if (scan->rs_pf_bhs_iterator) - { - bhs_end_iterate(scan->rs_pf_bhs_iterator); - scan->rs_pf_bhs_iterator = NULL; - } } scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 60916bf0d0..430668f597 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1758,11 +1758,7 @@ typedef enum /* ---------------- * ParallelBitmapHeapState information * tbmiterator iterator for scanning current pages - * prefetch_iterator iterator for prefetching ahead of current page - * mutex mutual exclusion for the prefetching variable - * and state - * prefetch_pages # pages prefetch iterator is ahead of current - * prefetch_target current target prefetch distance + * mutex mutual exclusion for state * state current state of the TIDBitmap * cv conditional wait variable * ---------------- @@ -1770,10 +1766,7 @@ typedef enum typedef struct ParallelBitmapHeapState { dsa_pointer tbmiterator; - dsa_pointer prefetch_iterator; slock_t mutex; - int prefetch_pages; - int prefetch_target; SharedBitmapState state; ConditionVariable cv; } ParallelBitmapHeapState; -- 2.40.1 --owzzsiozz6hgpp7e--