B-tree "finish incomplete split" bug - Mailing list pgsql-hackers

While testing my xlogutils.c refactoring patch, I bumped into an
existing bug in the B-tree code that finishes an incomplete split:

btree_xlog_cleanup() calls _bt_insert_parent() to insert the parent
pointer. If the split page was the root page, _bt_insert_parent()
creates a new root page by calling _bt_newroot(). _bt_newroot() calls
CacheInvalidateRelcache() to notify other backends, but
CacheInvalidateRelcache causes a segfault when called during WAL replay,
because the invalidation infrastructure hasn't been initialized yet.

This bug was introduced in 8.2, when we started to cache metapage
information in relcache. It's no wonder that no-one has bumped into this
in the field, as the window for that to happen is extremely small; I had
to inject an "XLogFlush(); elog(PANIC)" into _bt_split to trigger it. I
wish we had regression tests for WAL recovery :-(.

The trivial fix is to not call CacheInvalidateRelcache() in recovery
(patch attached). Another option is to put the check into
CacheInvalidateRelcache() itself, but in the name of consistency we
should then put the same check into the other CacheInvalidate* variants
as well. As nbtinsert.c is the only place that calls
CacheInvalidateRelcache during WAL replay, I'm going to do the trivial fix.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
Index: src/backend/access/nbtree/nbtinsert.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/nbtree/nbtinsert.c,v
retrieving revision 1.146.2.2
diff -c -r1.146.2.2 nbtinsert.c
*** src/backend/access/nbtree/nbtinsert.c    31 Dec 2007 04:52:20 -0000    1.146.2.2
--- src/backend/access/nbtree/nbtinsert.c    11 Jun 2008 08:20:25 -0000
***************
*** 690,696 ****
          /* release buffers; send out relcache inval if metapage changed */
          if (BufferIsValid(metabuf))
          {
!             CacheInvalidateRelcache(rel);
              _bt_relbuf(rel, metabuf);
          }

--- 690,697 ----
          /* release buffers; send out relcache inval if metapage changed */
          if (BufferIsValid(metabuf))
          {
!             if (!InRecovery)
!                 CacheInvalidateRelcache(rel);
              _bt_relbuf(rel, metabuf);
          }

***************
*** 1623,1629 ****
      END_CRIT_SECTION();

      /* send out relcache inval for metapage change */
!     CacheInvalidateRelcache(rel);

      /* done with metapage */
      _bt_relbuf(rel, metabuf);
--- 1627,1634 ----
      END_CRIT_SECTION();

      /* send out relcache inval for metapage change */
!     if (!InRecovery)
!         CacheInvalidateRelcache(rel);

      /* done with metapage */
      _bt_relbuf(rel, metabuf);