In SPGiST replay, do conflict resolution before modifying the page.

In yesterday's commit 962e0cc71e839c58fb9125fa85511b8bbb8bdbee, I added the
ResolveRecoveryConflictWithSnapshot call in the wrong place.  I correctly
put it before spgRedoVacuumRedirect itself would modify the index page ---
but not before RestoreBkpBlocks, so replay of a record with a full-page
image would modify the page before kicking off any conflicting HS
transactions.  Oops.

diff --git a/src/backend/access/spgist/spgxlog.c b/src/backend/access/spgist/spgxlog.c
index 45eda70ac765bdb39cba76d35127784c76821716..54e78f18b543b41b9a54294e5150f484275f3412 100644 (file)
--- a/src/backend/access/spgist/spgxlog.c
+++ b/src/backend/access/spgist/spgxlog.c
@@ -889,15 +889,6 @@ spgRedoVacuumRedirect(XLogRecPtr lsn, XLogRecord *record)
        ptr += sizeof(spgxlogVacuumRedirect);
        itemToPlaceholder = (OffsetNumber *) ptr;
 
-       /*
-        * If any redirection tuples are being removed, make sure there are no
-        * live Hot Standby transactions that might need to see them.  This code
-        * behaves similarly to btree's XLOG_BTREE_REUSE_PAGE case.
-        */
-       if (InHotStandby && TransactionIdIsValid(xldata->newestRedirectXid))
-               ResolveRecoveryConflictWithSnapshot(xldata->newestRedirectXid,
-                                                                                       xldata->node);
-
        if (!(record->xl_info & XLR_BKP_BLOCK_1))
        {
                buffer = XLogReadBuffer(xldata->node, xldata->blkno, false);
@@ -964,10 +955,33 @@ spg_redo(XLogRecPtr lsn, XLogRecord *record)
        MemoryContext oldCxt;
 
        /*
-        * SP-GiST indexes do not require any conflict processing. NB: If we ever
-        * implement a similar optimization as we have in b-tree, and remove
-        * killed tuples outside VACUUM, we'll need to handle that here.
+        * If we have any conflict processing to do, it must happen before we
+        * update the page.
         */
+       if (InHotStandby)
+       {
+               switch (info)
+               {
+                       case XLOG_SPGIST_VACUUM_REDIRECT:
+                               {
+                                       spgxlogVacuumRedirect *xldata =
+                                       (spgxlogVacuumRedirect *) XLogRecGetData(record);
+
+                                       /*
+                                        * If any redirection tuples are being removed, make sure
+                                        * there are no live Hot Standby transactions that might
+                                        * need to see them.
+                                        */
+                                       if (TransactionIdIsValid(xldata->newestRedirectXid))
+                                               ResolveRecoveryConflictWithSnapshot(xldata->newestRedirectXid,
+                                                                                                                       xldata->node);
+                                       break;
+                               }
+                       default:
+                               break;
+               }
+       }
+
        RestoreBkpBlocks(lsn, record, false);
 
        oldCxt = MemoryContextSwitchTo(opCtx);
@@ -1072,7 +1086,7 @@ spg_desc(StringInfo buf, uint8 xl_info, char *rec)
                        out_target(buf, ((spgxlogVacuumRedirect *) rec)->node);
                        appendStringInfo(buf, "vacuum redirect tuples on page %u, newest XID %u",
                                                         ((spgxlogVacuumRedirect *) rec)->blkno,
-                                                        ((spgxlogVacuumRedirect *) rec)->newestRedirectXid);
+                                                ((spgxlogVacuumRedirect *) rec)->newestRedirectXid);
                        break;
                default:
                        appendStringInfo(buf, "unknown spgist op code %u", info);