Thread: pgsql: Avoid pin scan for replay of XLOG_BTREE_VACUUM

Avoid pin scan for replay of XLOG_BTREE_VACUUM
Replay of XLOG_BTREE_VACUUM during Hot Standby was previously thought to require
complex interlocking that matched the requirements on the master. This required
an O(N) operation that became a significant problem with large indexes, causing
replication delays of seconds or in some cases minutes while the
XLOG_BTREE_VACUUM was replayed.

This commit skips the “pin scan” that was previously required, by observing in
detail when and how it is safe to do so, with full documentation. The pin scan
is skipped only in replay; the VACUUM code path on master is not touched here.

The current commit still performs the pin scan for toast indexes, though this
can also be avoided if we recheck scans on toast indexes. Later patch will
address this.

No tests included. Manual tests using an additional patch to view WAL records
and their timing have shown the change in WAL records and their handling has
successfully reduced replication delay.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/687f2cd7a0150647794efe432ae0397cb41b60ff

Modified Files
--------------
src/backend/access/nbtree/README      |   24 ++++++++++++++++++++++++
src/backend/access/nbtree/nbtree.c    |   23 ++++++++++++++++++++++-
src/backend/access/nbtree/nbtxlog.c   |   18 ++++++++++++++++--
src/backend/access/rmgrdesc/nbtdesc.c |    2 +-
src/include/access/nbtree.h           |    6 ++++--
5 files changed, 67 insertions(+), 6 deletions(-)

Hi,

On 2016-01-09 10:13:27 +0000, Simon Riggs wrote:
> src/backend/access/rmgrdesc/nbtdesc.c |    2 +-

I've not reviewed the patch, but a very quick glance during a rebase
with conflicts showed:

@@ -48,7 +48,7 @@ btree_desc(StringInfo buf, XLogReaderState *record)
            {
                xl_btree_vacuum *xlrec = (xl_btree_vacuum *) rec;

-               appendStringInfo(buf, "lastBlockVacuumed %u",
+               appendStringInfo(buf, "lastBlockVacuumed %d",
                                 xlrec->lastBlockVacuumed);
                break;
            }

which doesn't look right?

On 2016-01-09 17:58:01 +0000, Simon Riggs wrote:
> On 9 January 2016 at 12:23, Andres Freund <andres@anarazel.de> wrote:
>
> > Hi,
> >
> > On 2016-01-09 10:13:27 +0000, Simon Riggs wrote:
> > > src/backend/access/rmgrdesc/nbtdesc.c |    2 +-
> >
> > I've not reviewed the patch, but a very quick glance during a rebase
> > with conflicts showed:
> >
> > @@ -48,7 +48,7 @@ btree_desc(StringInfo buf, XLogReaderState *record)
> >             {
> >                 xl_btree_vacuum *xlrec = (xl_btree_vacuum *) rec;
> >
> > -               appendStringInfo(buf, "lastBlockVacuumed %u",
> > +               appendStringInfo(buf, "lastBlockVacuumed %d",
> >                                  xlrec->lastBlockVacuumed);
> >                 break;
> >             }
> >
> > which doesn't look right?
> >
>
> It's right. New value of -1 allowed in that field, so change required to
> allow it to display properly for debug.

Uh. xl_btree_vacuum->lastBlockVacuumed is of type BlockNumber, which in
turn is of type uint32. So no, this isn't correct as is.