agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH v13 04/16] BitmapPrefetch use prefetch block recheck for skip fetch 23+ messages / 1 participants [nested] [flat]
* [PATCH v3 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --fa5b4qdlc6zixc7z Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v4 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --5aaqsqqhbq27q3qo Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v5 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --6kjpcnqyi64ibp5i Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v13 04/16] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --cuuqjeyokkhgd736 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v15 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it could skip prefetching the proposed prefetch block. This doesn't seem right. We should use the prefetch block's recheck flag to determine whether or not to prefetch it. The current block's recheck flag has no relationship to the prefetch block's recheck flag. See this [1] thread on hackers reporting the issue. [1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.g... Author: Melanie Plageman Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/executor/nodeBitmapHeapscan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index a8bc5dec53..2e2cec8b3b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * skip this prefetch call, but continue to run the prefetch * logic normally. (Would it be better not to increment * prefetch_pages?) - * - * This depends on the assumption that the index AM will - * report the same recheck flag for this future heap page as - * it did for the current heap page; which is not a certainty - * but is true in many cases. */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --pbix6fw3h4kvmjae Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v16 04/18] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it could skip prefetching the proposed prefetch block. This doesn't seem right. We should use the prefetch block's recheck flag to determine whether or not to prefetch it. The current block's recheck flag has no relationship to the prefetch block's recheck flag. See this [1] thread on hackers reporting the issue. [1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.g... Author: Melanie Plageman Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/executor/nodeBitmapHeapscan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index a8bc5dec53..2e2cec8b3b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * skip this prefetch call, but continue to run the prefetch * logic normally. (Would it be better not to increment * prefetch_pages?) - * - * This depends on the assumption that the index AM will - * report the same recheck flag for this future heap page as - * it did for the current heap page; which is not a certainty - * but is true in many cases. */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --e3xl7h75mzefinno Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v6 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index df382580ac1..6a34f10646a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -470,15 +470,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -529,7 +523,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v7 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 81e3166d51b..dc8c312cafb 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -474,15 +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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v8 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v9 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v10 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v11 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v13 04/16] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --cuuqjeyokkhgd736 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v15 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it could skip prefetching the proposed prefetch block. This doesn't seem right. We should use the prefetch block's recheck flag to determine whether or not to prefetch it. The current block's recheck flag has no relationship to the prefetch block's recheck flag. See this [1] thread on hackers reporting the issue. [1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.g... Author: Melanie Plageman Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/executor/nodeBitmapHeapscan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index a8bc5dec53..2e2cec8b3b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * skip this prefetch call, but continue to run the prefetch * logic normally. (Would it be better not to increment * prefetch_pages?) - * - * This depends on the assumption that the index AM will - * report the same recheck flag for this future heap page as - * it did for the current heap page; which is not a certainty - * but is true in many cases. */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --pbix6fw3h4kvmjae Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v16 04/18] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it could skip prefetching the proposed prefetch block. This doesn't seem right. We should use the prefetch block's recheck flag to determine whether or not to prefetch it. The current block's recheck flag has no relationship to the prefetch block's recheck flag. See this [1] thread on hackers reporting the issue. [1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.g... Author: Melanie Plageman Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/executor/nodeBitmapHeapscan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index a8bc5dec53..2e2cec8b3b 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * skip this prefetch call, but continue to run the prefetch * logic normally. (Would it be better not to increment * prefetch_pages?) - * - * This depends on the assumption that the index AM will - * report the same recheck flag for this future heap page as - * it did for the current heap page; which is not a certainty - * but is true in many cases. */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --e3xl7h75mzefinno Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v3 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --fa5b4qdlc6zixc7z Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v4 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --5aaqsqqhbq27q3qo Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v5 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 2e4f87ea3a3..35ef26221ba 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -484,15 +484,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -543,7 +537,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.37.2 --6kjpcnqyi64ibp5i Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v6 04/14] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index df382580ac1..6a34f10646a 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -470,15 +470,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -529,7 +523,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (scan->rs_flags & SO_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v7 04/13] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- src/backend/executor/nodeBitmapHeapscan.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 81e3166d51b..dc8c312cafb 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -474,15 +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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,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_CAN_SKIP_FETCH && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v8 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v9 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0005-Update-BitmapAdjustPrefetchIterator-parameter-typ.patch" ^ permalink raw reply [nested|flat] 23+ messages in thread
* [PATCH v11 04/17] BitmapPrefetch use prefetch block recheck for skip fetch @ 2024-02-13 00:03 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 23+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) 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.g... --- 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" ^ permalink raw reply [nested|flat] 23+ messages in thread
end of thread, other threads:[~2024-02-13 00:03 UTC | newest] Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-02-13 00:03 [PATCH v3 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v4 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v5 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v13 04/16] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v15 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v16 04/18] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v6 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v7 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v8 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v9 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v10 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v11 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v13 04/16] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v15 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v16 04/18] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v3 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v4 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v5 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v6 04/14] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v7 04/13] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v8 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v9 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com> 2024-02-13 00:03 [PATCH v11 04/17] BitmapPrefetch use prefetch block recheck for skip fetch Melanie Plageman <melanieplageman@gmail.com>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox