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);