Avoid palloc in critical section in GiST WAL-logging.

Memory allocation can fail if you run out of memory, and inside a critical
section that will lead to a PANIC. Use conservatively-sized arrays in stack
instead.

There was previously no explicit limit on the number of pages a GiST split
can produce, it was only limited by the number of LWLocks that can be held
simultaneously (100 at the moment). This patch adds an explicit limit of 75
pages. That should be plenty, a typical split shouldn't produce more than
2-3 page halves.

The bug has been there forever, but only backpatch down to 9.1. The code
was changed significantly in 9.1, and it doesn't seem worth the risk or
trouble to adapt this for 9.0 and 8.4.

diff --git a/src/backend/access/gist/README b/src/backend/access/gist/README
index 2d78dcb0dfaf21e348bcca3da8ba3829bcac5288..b24673e624a3162c4fd77dd1237f8e23f5852893 100644 (file)
--- a/src/backend/access/gist/README
+++ b/src/backend/access/gist/README
@@ -128,7 +128,7 @@ that didn't need to be split.
 
 This differs from the insertion algorithm in the original paper. In the
 original paper, you first walk down the tree until you reach a leaf page, and
-then you adjust the downlink in the parent, and propagating the adjustment up,
+then you adjust the downlink in the parent, and propagate the adjustment up,
 all the way up to the root in the worst case. But we adjust the downlinks to
 cover the new key already when we walk down, so that when we reach the leaf
 page, we don't need to update the parents anymore, except to insert the

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 7dbc5dd08ec3b95d3d607583c272c3742f75c6b4..7172f21028615dba02bc6d76bd7aab9828ec9775 100644 (file)
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -356,6 +356,7 @@ gistplacetopage(GISTInsertState *state, GISTSTATE *giststate,
                SplitedPageLayout rootpg;
                BlockNumber blkno = BufferGetBlockNumber(buffer);
                bool            is_rootsplit;
+               int                     npage;
 
                is_rootsplit = (blkno == GIST_ROOT_BLKNO);
 
@@ -376,6 +377,19 @@ gistplacetopage(GISTInsertState *state, GISTSTATE *giststate,
                itvec = gistjoinvector(itvec, &tlen, itup, ntup);
                dist = gistSplit(state->r, page, itvec, tlen, giststate);
 
+               /*
+                * Check that split didn't produce too many pages.
+                */
+               npage = 0;
+               for (ptr = dist; ptr; ptr = ptr->next)
+                       npage++;
+               /* in a root split, we'll add one more page to the list below */
+               if (is_rootsplit)
+                       npage++;
+               if (npage > GIST_MAX_SPLIT_PAGES)
+                       elog(ERROR, "GiST page split into too many halves (%d, maximum %d)",
+                                npage, GIST_MAX_SPLIT_PAGES);
+
                /*
                 * Set up pages to work with. Allocate new buffers for all but the
                 * leftmost page. The original page becomes the new leftmost page, and

diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c
index b7de92c8865622380c81b8b21ff895463d88fe20..d0ca2ffa4391e681e7ca7208130a8dd3a0777cfe 100644 (file)
--- a/src/backend/access/gist/gistxlog.c
+++ b/src/backend/access/gist/gistxlog.c
@@ -434,7 +434,7 @@ gistXLogSplit(RelFileNode node, BlockNumber blkno, bool page_is_leaf,
                          BlockNumber origrlink, GistNSN orignsn,
                          Buffer leftchildbuf)
 {
-       XLogRecData *rdata;
+       XLogRecData rdata[GIST_MAX_SPLIT_PAGES * 2 + 2];
        gistxlogPageSplit xlrec;
        SplitedPageLayout *ptr;
        int                     npage = 0,
@@ -443,8 +443,12 @@ gistXLogSplit(RelFileNode node, BlockNumber blkno, bool page_is_leaf,
 
        for (ptr = dist; ptr; ptr = ptr->next)
                npage++;
-
-       rdata = (XLogRecData *) palloc(sizeof(XLogRecData) * (npage * 2 + 2));
+       /*
+        * the caller should've checked this already, but doesn't hurt to check
+        * again.
+        */
+       if (npage > GIST_MAX_SPLIT_PAGES)
+               elog(ERROR, "GiST page split into too many halves");
 
        xlrec.node = node;
        xlrec.origblkno = blkno;
@@ -493,7 +497,6 @@ gistXLogSplit(RelFileNode node, BlockNumber blkno, bool page_is_leaf,
 
        recptr = XLogInsert(RM_GIST_ID, XLOG_GIST_PAGE_SPLIT, rdata);
 
-       pfree(rdata);
        return recptr;
 }
 
@@ -516,14 +519,12 @@ gistXLogUpdate(RelFileNode node, Buffer buffer,
                           IndexTuple *itup, int ituplen,
                           Buffer leftchildbuf)
 {
-       XLogRecData *rdata;
+       XLogRecData rdata[MaxIndexTuplesPerPage + 3];
        gistxlogPageUpdate xlrec;
        int                     cur,
                                i;
        XLogRecPtr      recptr;
 
-       rdata = (XLogRecData *) palloc(sizeof(XLogRecData) * (3 + ituplen));
-
        xlrec.node = node;
        xlrec.blkno = BufferGetBlockNumber(buffer);
        xlrec.ntodelete = ntodelete;
@@ -570,6 +571,5 @@ gistXLogUpdate(RelFileNode node, Buffer buffer,
 
        recptr = XLogInsert(RM_GIST_ID, XLOG_GIST_PAGE_UPDATE, rdata);
 
-       pfree(rdata);
        return recptr;
 }

diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index 9810c2d1dd3875f9d06174f07712bdc7ec2cb808..1da9dfaeb4488efc20a786fbe269de75cc04eee4 100644 (file)
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -19,6 +19,21 @@
 #include "storage/bufmgr.h"
 #include "utils/rbtree.h"
 
+/*
+ * Maximum number of "halves" a page can be split into in one operation.
+ * Typically a split produces 2 halves, but can be more if keys have very
+ * different lengths, or when inserting multiple keys in one operation (as
+ * when inserting downlinks to an internal node).  There is no theoretical
+ * limit on this, but in practice if you get more than a handful page halves
+ * in one split, there's something wrong with the opclass implementation.
+ * GIST_MAX_SPLIT_PAGES is an arbitrary limit on that, used to size some
+ * local arrays used during split.  Note that there is also a limit on the
+ * number of buffers that can be held locked at a time, MAX_SIMUL_LWLOCKS,
+ * so if you raise this higher than that limit, you'll just get a different
+ * error.
+ */
+#define GIST_MAX_SPLIT_PAGES           75
+
 /* Buffer lock modes */
 #define GIST_SHARE     BUFFER_LOCK_SHARE
 #define GIST_EXCLUSIVE BUFFER_LOCK_EXCLUSIVE