Обсуждение: B-tree "finish incomplete split" bug

Поиск
Список
Период
Сортировка

B-tree "finish incomplete split" bug

От
"Heikki Linnakangas"
Дата:
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);

Re: B-tree "finish incomplete split" bug

От
Tom Lane
Дата:
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> 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.

This will need to be revisited if we ever hope to get read-only slaves
working.  But I agree with the trivial fix for now (especially in the
back branches).
        regards, tom lane


Re: B-tree "finish incomplete split" bug

От
Alvaro Herrera
Дата:
Tom Lane wrote:
> "Heikki Linnakangas" <heikki@enterprisedb.com> writes:
> > 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.
> 
> This will need to be revisited if we ever hope to get read-only slaves
> working.  But I agree with the trivial fix for now (especially in the
> back branches).

Is this being handled by the hot standby patch?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support