From 5b353aaff69c6099aebd45f1a63cb38b9a7ef75c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 18 Oct 2021 11:56:48 +0900 Subject: [PATCH] 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 4cc38f0d85..d96881c0de 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -46,6 +46,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" @@ -2698,6 +2699,9 @@ AbortTransaction(void) /* Reset logical streaming state. */ ResetLogicalStreamingState(); + /* Reset snapshot export state. */ + SnapBuildResetExportedSnapshotState(); + /* If in parallel mode, clean up workers and exit parallel mode. */ if (IsInParallelMode()) { @@ -5010,6 +5014,11 @@ AbortSubTransaction(void) /* Reset logical streaming state. */ ResetLogicalStreamingState(); + /* + * 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 04f3355f60..6df602485b 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -673,6 +673,8 @@ SnapBuildGetOrBuildSnapshot(SnapBuild *builder, TransactionId xid) void SnapBuildClearExportedSnapshot(void) { + ResourceOwner tmpResOwner; + /* nothing exported, that is the usual case */ if (!ExportInProgress) return; @@ -680,10 +682,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 fbabce6764..3604621e88 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -70,6 +70,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,