Re: Hot standby 9.2.6 -> 9.2.6 PANIC: WAL contains references to invalid pages

On 01/14/2014 03:59 AM, Tom Lane wrote:
> BTW, while I'm looking at this ... the writing side of the code seems a
> few bricks shy of a load too:
>
>      /*
>       * InHotStandby we need to scan right up to the end of the index for
>       * correct locking, so we may need to write a WAL record for the final
>       * block in the index if it was not vacuumed. It's possible that VACUUMing
>       * has actually removed zeroed pages at the end of the index so we need to
>       * take care to issue the record for last actual block and not for the
>       * last block that was scanned. Ignore empty indexes.
>       */
>      if (XLogStandbyInfoActive() &&
>          num_pages > 1 && vstate.lastBlockVacuumed < (num_pages - 1))
>      {
>          Buffer        buf;
>
>          /*
>           * We can't use _bt_getbuf() here because it always applies
>           * _bt_checkpage(), which will barf on an all-zero page. We want to
>           * recycle all-zero pages, not fail.  Also, we want to use a
>           * nondefault buffer access strategy.
>           */
>          buf = ReadBufferExtended(rel, MAIN_FORKNUM, num_pages - 1, RBM_NORMAL,
>                                   info->strategy);
>          LockBufferForCleanup(buf);
>          _bt_delitems_vacuum(rel, buf, NULL, 0, vstate.lastBlockVacuumed);
>          _bt_relbuf(rel, buf);
>      }
>
> If the last physical page of the index is all-zero, the preceding loop
> won't have any problem with that (nor should it); but this code sure will
> have a problem, because _bt_delitems_vacuum isn't prepared to cope with an
> all-zero page AFAICS.

Yep, that's broken.

> Nor am I following the logic of the initial comment.  If we know that
> pages above N are all-zero, why are we worried about taking locks on them?
> There shouldn't be any scans reaching such pages, and any that did would
> error out anyhow in _bt_getbuf().

Also, btree relations are never truncated, so the comment is wrong when
it claims that VACUUM might've removed zero pages at the end of the index.

> ISTM the right thing here is for the btvacuumpage loop to remember the
> last ordinary, valid index page it saw, and point to that one in this
> added WAL entry.

Agreed.

- Heikki