Reset properly snapshot export state during transaction abort

During a replication slot creation, an ERROR generated in the same
transaction as the one creating a to-be-exported snapshot would have
left the backend in an inconsistent state, as the associated static
export snapshot state was not being reset on transaction abort, but only
on the follow-up command received by the WAL sender that created this
snapshot on replication slot creation.  This would trigger inconsistency
failures if this session tried to export again a snapshot, like during
the creation of a replication slot.

Note that a snapshot export cannot happen in a transaction block, so
there is no need to worry resetting this state for subtransaction
aborts.  Also, this inconsistent state would very unlikely show up to
users.  For example, one case where this could happen is an
out-of-memory error when building the initial snapshot to-be-exported.
Dilip found this problem while poking at a different patch, that caused
an error in this code path for reasons unrelated to HEAD.

Author: Dilip Kumar
Reviewed-by: Michael Paquier, Zhihong Yu
Discussion: https://postgr.es/m/CAFiTN-s0zA1Kj0ozGHwkYkHwa5U0zUE94RSc_g81WrpcETB5=w@mail.gmail.com
Backpatch-through: 9.6

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 8815127781809dcc695b705c00a16c0700bb40a2..5e9dbdad584e57ee4dae9ddfda4e045ef6d1bc1b 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -44,6 +44,7 @@
 #include "replication/logical.h"
 #include "replication/logicallauncher.h"
 #include "replication/origin.h"
+#include "replication/snapbuild.h"
 #include "replication/syncrep.h"
 #include "replication/walsender.h"
 #include "storage/condition_variable.h"
@@ -2581,6 +2582,9 @@ AbortTransaction(void)
        /* Forget about any active REINDEX. */
        ResetReindexState(s->nestingLevel);
 
+       /* Reset snapshot export state. */
+       SnapBuildResetExportedSnapshotState();
+
        /* If in parallel mode, clean up workers and exit parallel mode. */
        if (IsInParallelMode())
        {
@@ -4794,6 +4798,11 @@ AbortSubTransaction(void)
        /* Forget about any active REINDEX. */
        ResetReindexState(s->nestingLevel);
 
+       /*
+        * No need for SnapBuildResetExportedSnapshotState() here, snapshot
+        * exports are not supported in subtransactions.
+        */
+
        /* Exit from parallel mode, if necessary. */
        if (IsInParallelMode())
        {

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 521e5d238f1b5263a0367ed73a6ce42a33ae7fa4..1c52bc64e39b89c15163b1b87a824140d64e36a3 100644 (file)
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -697,6 +697,8 @@ SnapBuildGetOrBuildSnapshot(SnapBuild *builder, TransactionId xid)
 void
 SnapBuildClearExportedSnapshot(void)
 {
+       ResourceOwner tmpResOwner;
+
        /* nothing exported, that is the usual case */
        if (!ExportInProgress)
                return;
@@ -704,10 +706,24 @@ SnapBuildClearExportedSnapshot(void)
        if (!IsTransactionState())
                elog(ERROR, "clearing exported snapshot in wrong transaction state");
 
-       /* make sure nothing  could have ever happened */
+       /*
+        * AbortCurrentTransaction() takes care of resetting the snapshot state,
+        * so remember SavedResourceOwnerDuringExport.
+        */
+       tmpResOwner = SavedResourceOwnerDuringExport;
+
+       /* make sure nothing could have ever happened */
        AbortCurrentTransaction();
 
-       CurrentResourceOwner = SavedResourceOwnerDuringExport;
+       CurrentResourceOwner = tmpResOwner;
+}
+
+/*
+ * Clear snapshot export state during transaction abort.
+ */
+void
+SnapBuildResetExportedSnapshotState(void)
+{
        SavedResourceOwnerDuringExport = NULL;
        ExportInProgress = false;
 }

diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h
index 56257430aea9b1af5214d0ef53fe5a9856f3a1ea..1df66a3c75df1da391fff29f06db4d849f6f1a14 100644 (file)
--- a/src/include/replication/snapbuild.h
+++ b/src/include/replication/snapbuild.h
@@ -69,6 +69,7 @@ extern void SnapBuildSnapDecRefcount(Snapshot snap);
 extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder);
 extern const char *SnapBuildExportSnapshot(SnapBuild *snapstate);
 extern void SnapBuildClearExportedSnapshot(void);
+extern void SnapBuildResetExportedSnapshotState(void);
 
 extern SnapBuildState SnapBuildCurrentState(SnapBuild *snapstate);
 extern Snapshot SnapBuildGetOrBuildSnapshot(SnapBuild *builder,