nbtree VACUUM: cope with right sibling link corruption.

Avoid "right sibling's left-link doesn't match" errors when vacuuming a
corrupt nbtree index.  Just LOG the issue and press on.  That way VACUUM
will have a decent chance of finishing off all required processing for
the index (and for the table as a whole).

This error was seen in the field from time to time (it's more than a
theoretical risk), so giving VACUUM the ability to press on like this
has real value.  Nothing short of a REINDEX is expected to fix the
underlying index corruption, so giving up (by throwing an error) risks
making a bad situation far worse.  Anything that blocks forward progress
by VACUUM like this might go unnoticed for a long time.  This could
eventually lead to a wraparound/xidStopLimit outage.

Note that _bt_unlink_halfdead_page() has always been able to bail on
page deletion when the target page's left sibling page was in an
inconsistent state.  It now does the same thing (returns false to back
out of the second phase of deletion) when it notices sibling link
corruption in the target page's right sibling page.

This is similar to the work from commit 5b861baa (later backpatched as
commit 43e409ce), which taught nbtree to press on with vacuuming an
index when page deletion fails to "re-find" a downlink in the target
page's parent page.  The "re-find" check seems to make VACUUM bail on
page deletion more often in practice, but there is no reason to take any
chances here.

Author: Peter Geoghegan <[email protected]>
Reviewed-By: Heikki Linnakangas <[email protected]>
Discussion: https://postgr.es/m/CAH2-Wzko2q2kP1+UvgJyP9g0mF4hopK0NtQZcxwvMv9_ytGhkQ@mail.gmail.com
Backpatch: 11- (all supported versions).

diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 6be8915229e7185b3231b262ba01ff8838621a94..486d1563d8a479cc106c99f3ec898c58f33ce81c 100644 (file)
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2404,24 +2404,22 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
 
            if (!leftsibvalid)
            {
-               if (target != leafblkno)
-               {
-                   /* we have only a pin on target, but pin+lock on leafbuf */
-                   ReleaseBuffer(buf);
-                   _bt_relbuf(rel, leafbuf);
-               }
-               else
-               {
-                   /* we have only a pin on leafbuf */
-                   ReleaseBuffer(leafbuf);
-               }
-
+               /*
+                * This is known to fail in the field; sibling link corruption
+                * is relatively common.  Press on with vacuuming rather than
+                * just throwing an ERROR.
+                */
                ereport(LOG,
                        (errcode(ERRCODE_INDEX_CORRUPTED),
                         errmsg_internal("valid left sibling for deletion target could not be located: "
-                                        "left sibling %u of target %u with leafblkno %u and scanblkno %u in index \"%s\"",
+                                        "left sibling %u of target %u with leafblkno %u and scanblkno %u on level %u of index \"%s\"",
                                         leftsib, target, leafblkno, scanblkno,
-                                        RelationGetRelationName(rel))));
+                                        targetlevel, RelationGetRelationName(rel))));
+
+               /* Must release all pins and locks on failure exit */
+               ReleaseBuffer(buf);
+               if (target != leafblkno)
+                   _bt_relbuf(rel, leafbuf);
 
                return false;
            }
@@ -2496,13 +2494,40 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
    rbuf = _bt_getbuf(rel, vstate->info->heaprel, rightsib, BT_WRITE);
    page = BufferGetPage(rbuf);
    opaque = BTPageGetOpaque(page);
+
+   /*
+    * Validate target's right sibling page.  Its left link must point back to
+    * the target page.
+    */
    if (opaque->btpo_prev != target)
-       ereport(ERROR,
+   {
+       /*
+        * This is known to fail in the field; sibling link corruption is
+        * relatively common.  Press on with vacuuming rather than just
+        * throwing an ERROR (same approach used for left-sibling's-right-link
+        * validation check a moment ago).
+        */
+       ereport(LOG,
                (errcode(ERRCODE_INDEX_CORRUPTED),
                 errmsg_internal("right sibling's left-link doesn't match: "
-                                "block %u links to %u instead of expected %u in index \"%s\"",
-                                rightsib, opaque->btpo_prev, target,
-                                RelationGetRelationName(rel))));
+                                "right sibling %u of target %u with leafblkno %u "
+                                "and scanblkno %u spuriously links to non-target %u "
+                                "on level %u of index \"%s\"",
+                                rightsib, target, leafblkno,
+                                scanblkno, opaque->btpo_prev,
+                                targetlevel, RelationGetRelationName(rel))));
+
+       /* Must release all pins and locks on failure exit */
+       if (BufferIsValid(lbuf))
+           _bt_relbuf(rel, lbuf);
+       _bt_relbuf(rel, rbuf);
+       _bt_relbuf(rel, buf);
+       if (target != leafblkno)
+           _bt_relbuf(rel, leafbuf);
+
+       return false;
+   }
+
    rightsib_is_rightmost = P_RIGHTMOST(opaque);
    *rightsib_empty = (P_FIRSTDATAKEY(opaque) > PageGetMaxOffsetNumber(page));
 
@@ -2727,6 +2752,7 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
     */
    _bt_pendingfsm_add(vstate, target, safexid);
 
+   /* Success - hold on to lock on leafbuf (might also have been target) */
    return true;
 }
 

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 2df8849858e6e8a903d405384ece4e97e7369b0e..1ce5b15199a9a101464c7d331877275ffb230135 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -1093,8 +1093,7 @@ backtrack:
         * can't be half-dead because only an interrupted VACUUM process can
         * leave pages in that state, so we'd definitely have dealt with it
         * back when the page was the scanblkno page (half-dead pages are
-        * always marked fully deleted by _bt_pagedel()).  This assumes that
-        * there can be only one vacuum process running at a time.
+        * always marked fully deleted by _bt_pagedel(), barring corruption).
         */
        if (!opaque || !P_ISLEAF(opaque) || P_ISHALFDEAD(opaque))
        {