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.

diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 42fe9554f0f5fc075164f177ae98a8c1ae72a9e1..58cccb6e7244a7ba5b0aafb20d9e112042304ced 100644 (file)
--- 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 c21bc281778876661c3b4e3137d0e0233926314c..5a9763fe9a1709ed77ae8447207bc8ee88c358ce 100644 (file)
--- 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 b8bb1ad4906d5fb1dfe504f9e345fa53fe9b5061..a90a0b183409c793534772970804b3ba775e3b50 100644 (file)
--- 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 75f49ee510812add0bd3bb1928cd0960d315a080..78b47000621a87388138d80bbfc6e9b260dadfb3 100644 (file)
--- 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);