From 8f348112f35d9dcc28fc575f8bae458883c5700a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 5 May 2009 19:36:32 +0000 Subject: [PATCH] Insert CHECK_FOR_INTERRUPTS() calls into btree and hash index scans at the points where we step right or left to the next page. This should ensure reasonable response time to a query cancel request during an unsuccessful index scan, as seen in recent gripe from Marc Cousin. It's a bit trickier than it might seem at first glance, because CHECK_FOR_INTERRUPTS() is a no-op if executed while holding a buffer lock. So we have to do it just at the point where we've dropped one page lock and not yet acquired the next. Remove CHECK_FOR_INTERRUPTS calls at the top level of btgetbitmap and hashgetbitmap, since they're pointless given the added checks. I think that GIST is okay already --- at least, there's a CHECK_FOR_INTERRUPTS at a plausible-looking place in gistnext(). I don't claim to know GIN well enough to try to poke it for this, if indeed it has a problem at all. This is a pre-existing issue, but in view of the lack of prior complaints I'm not going to risk back-patching. --- src/backend/access/hash/hash.c | 5 +---- src/backend/access/hash/hashsearch.c | 7 ++++++- src/backend/access/nbtree/nbtree.c | 5 +---- src/backend/access/nbtree/nbtsearch.c | 22 +++++++++++++--------- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 42fe9554f0..58cccb6e72 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.109 2009/03/24 20:17:11 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.110 2009/05/05 19:36:32 tgl Exp $ * * NOTES * This file contains only the public interface routines. @@ -22,7 +22,6 @@ #include "access/relscan.h" #include "catalog/index.h" #include "commands/vacuum.h" -#include "miscadmin.h" #include "optimizer/cost.h" #include "optimizer/plancat.h" #include "storage/bufmgr.h" @@ -297,8 +296,6 @@ hashgetbitmap(PG_FUNCTION_ARGS) { bool add_tuple; - CHECK_FOR_INTERRUPTS(); - /* * Skip killed tuples if asked to. */ diff --git a/src/backend/access/hash/hashsearch.c b/src/backend/access/hash/hashsearch.c index c21bc28177..5a9763fe9a 100644 --- a/src/backend/access/hash/hashsearch.c +++ b/src/backend/access/hash/hashsearch.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.55 2009/01/01 17:23:35 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.56 2009/05/05 19:36:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -16,6 +16,7 @@ #include "access/hash.h" #include "access/relscan.h" +#include "miscadmin.h" #include "pgstat.h" #include "storage/bufmgr.h" #include "utils/rel.h" @@ -74,6 +75,8 @@ _hash_readnext(Relation rel, blkno = (*opaquep)->hasho_nextblkno; _hash_relbuf(rel, *bufp); *bufp = InvalidBuffer; + /* check for interrupts while we're not holding any buffer lock */ + CHECK_FOR_INTERRUPTS(); if (BlockNumberIsValid(blkno)) { *bufp = _hash_getbuf(rel, blkno, HASH_READ, LH_OVERFLOW_PAGE); @@ -94,6 +97,8 @@ _hash_readprev(Relation rel, blkno = (*opaquep)->hasho_prevblkno; _hash_relbuf(rel, *bufp); *bufp = InvalidBuffer; + /* check for interrupts while we're not holding any buffer lock */ + CHECK_FOR_INTERRUPTS(); if (BlockNumberIsValid(blkno)) { *bufp = _hash_getbuf(rel, blkno, HASH_READ, diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index b8bb1ad490..a90a0b1834 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -12,7 +12,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.168 2009/03/24 20:17:12 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.169 2009/05/05 19:36:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -24,7 +24,6 @@ #include "catalog/index.h" #include "catalog/storage.h" #include "commands/vacuum.h" -#include "miscadmin.h" #include "storage/bufmgr.h" #include "storage/freespace.h" #include "storage/indexfsm.h" @@ -315,8 +314,6 @@ btgetbitmap(PG_FUNCTION_ARGS) */ if (++so->currPos.itemIndex > so->currPos.lastItem) { - CHECK_FOR_INTERRUPTS(); - /* let _bt_next do the heavy lifting */ if (!_bt_next(scan, ForwardScanDirection)) break; diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c index 75f49ee510..78b4700062 100644 --- a/src/backend/access/nbtree/nbtsearch.c +++ b/src/backend/access/nbtree/nbtsearch.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.119 2009/01/01 17:23:35 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.120 2009/05/05 19:36:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -18,6 +18,7 @@ #include "access/genam.h" #include "access/nbtree.h" #include "access/relscan.h" +#include "miscadmin.h" #include "pgstat.h" #include "storage/bufmgr.h" #include "utils/lsyscache.h" @@ -1126,16 +1127,16 @@ _bt_steppage(IndexScanDesc scan, ScanDirection dir) for (;;) { - /* if we're at end of scan, release the buffer and return */ + /* release the previous buffer */ + _bt_relbuf(rel, so->currPos.buf); + so->currPos.buf = InvalidBuffer; + /* if we're at end of scan, give up */ if (blkno == P_NONE || !so->currPos.moreRight) - { - _bt_relbuf(rel, so->currPos.buf); - so->currPos.buf = InvalidBuffer; return false; - } + /* check for interrupts while we're not holding any buffer lock */ + CHECK_FOR_INTERRUPTS(); /* step right one page */ - so->currPos.buf = _bt_relandgetbuf(rel, so->currPos.buf, - blkno, BT_READ); + so->currPos.buf = _bt_getbuf(rel, blkno, BT_READ); /* check for deleted page */ page = BufferGetPage(so->currPos.buf); opaque = (BTPageOpaque) PageGetSpecialPointer(page); @@ -1239,7 +1240,10 @@ _bt_walk_left(Relation rel, Buffer buf) obknum = BufferGetBlockNumber(buf); /* step left */ blkno = lblkno = opaque->btpo_prev; - buf = _bt_relandgetbuf(rel, buf, blkno, BT_READ); + _bt_relbuf(rel, buf); + /* check for interrupts while we're not holding any buffer lock */ + CHECK_FOR_INTERRUPTS(); + buf = _bt_getbuf(rel, blkno, BT_READ); page = BufferGetPage(buf); opaque = (BTPageOpaque) PageGetSpecialPointer(page); -- GitLab