postgres.git / summary / log / commit / refs

commit    a207b8521346bf9575d7d0c7585a3de8250d4ef8
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Mon Oct 18 02:56:54 2021 +0000
First released in: 15.0, 9.6.24, 10.19, 11.14, 12.9, 13.5, 14.1

    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


 src/backend/access/transam/xact.c           |  9 +++++++++
 src/backend/replication/logical/snapbuild.c | 20 ++++++++++++++++++--
 src/include/replication/snapbuild.h         |  1 +
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index ad465d8eae2..b7aaea202c0 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -45,6 +45,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"
@@ -2651,6 +2652,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())
 	{
@@ -4962,6 +4966,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 4ec0003550e..5a1bce5acc0 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -696,6 +696,8 @@ SnapBuildGetOrBuildSnapshot(SnapBuild *builder, TransactionId xid)
 void
 SnapBuildClearExportedSnapshot(void)
 {
+	ResourceOwner tmpResOwner;
+
 	/* nothing exported, that is the usual case */
 	if (!ExportInProgress)
 		return;
@@ -703,10 +705,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 392fe1609ba..3acf68f5bdf 100644
--- 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,


[parent: 5b0b2983a095]