v52-3-0001-Don-t-lose-a-conflict-when-the-conflict-log-table.txt

From e5954eb1ef36a98b7dcc31697dd2427070892de3 Mon Sep 17 00:00:00 2001
From: Amit Kapila <akapila@postgresql.org>
Date: Mon, 22 Jun 2026 12:18:17 +0530
Subject: [PATCH v3] Don't lose a conflict when the conflict log table is
 dropped mid-logging

A concurrent ALTER SUBSCRIPTION changing conflict_log_destination can drop the
conflict log table while a conflict is being logged, making table_open() fail
and the conflict lost from both the table and the server log.  Open it with
try_table_open() instead and, on NULL, fall back to full server-log reporting.
---
 src/backend/replication/logical/conflict.c | 108 +++++++++++++++------
 src/include/replication/conflict.h         |   3 +-
 2 files changed, 82 insertions(+), 29 deletions(-)

diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index c2c15f055e6..983cdf94cf0 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -319,7 +319,6 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
 					TupleTableSlot *remoteslot, List *conflicttuples)
 {
 	Relation		localrel = relinfo->ri_RelationDesc;
-	ConflictLogDest	dest;
 	Relation		conflictlogrel;
 	bool			log_dest_table;
 	bool 			log_dest_logfile;
@@ -327,13 +326,14 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
 	pgstat_report_subscription_conflict(MySubscription->oid, type);
 
 	/*
-	 * Get the conflict log destination. Also, (if there is one) return the
-	 * CLT relation already opened and ready for insertion.
+	 * Get the conflict log destinations.  Also, if a table is one of the
+	 * destinations, return the CLT relation already opened and ready for
+	 * insertion.  If the table was requested but has been dropped concurrently,
+	 * GetConflictLogDestAndTable() returns NULL and reports server-log as the
+	 * destination instead, so the conflict is still recorded.
 	 */
-	conflictlogrel = GetConflictLogDestAndTable(&dest);
-
-	log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
-	log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
+	conflictlogrel = GetConflictLogDestAndTable(&log_dest_table,
+												&log_dest_logfile);
 
 	/*
 	 * Prepare the conflict log tuple first when the destination includes the
@@ -466,7 +466,6 @@ void
 ProcessPendingConflictLogTuple(void)
 {
 	Relation	conflictlogrel;
-	ConflictLogDest dest;
 
 	/* Nothing to do */
 	if (MyLogicalRepWorker->conflict_log_tuple == NULL)
@@ -474,23 +473,40 @@ ProcessPendingConflictLogTuple(void)
 
 	/*
 	 * Insert the deferred conflict log tuple in its own transaction.  A
-	 * failure here (e.g. the conflict log table was dropped, or an
-	 * out-of-disk-space error) is treated like any other apply error and
-	 * raises an ERROR; such failures are expected to be rare and persistent.
-	 * Callers must therefore have already reported (and cleared) any
-	 * in-progress apply error before calling this, so that this error does not
-	 * mask the original one.
+	 * failure here (e.g. an out-of-disk-space error) is treated like any other
+	 * apply error and raises an ERROR; such failures are expected to be rare
+	 * and persistent.  Callers must therefore have already reported (and
+	 * cleared) any in-progress apply error before calling this, so that this
+	 * error does not mask the original one.
 	 */
 	StartTransactionCommand();
 	PushActiveSnapshot(GetTransactionSnapshot());
 
-	/* Open conflict log table and insert the tuple */
-	conflictlogrel = GetConflictLogDestAndTable(&dest);
-	Assert(conflictlogrel);
+	/* Open the conflict log table and insert the tuple. */
+	conflictlogrel = GetConflictLogDestAndTable(NULL, NULL);
 
-	InsertConflictLogTuple(conflictlogrel);
-
-	table_close(conflictlogrel, RowExclusiveLock);
+	if (conflictlogrel != NULL)
+	{
+		InsertConflictLogTuple(conflictlogrel);
+		table_close(conflictlogrel, RowExclusiveLock);
+	}
+	else
+	{
+		/*
+		 * The conflict log table was dropped concurrently (e.g. by an ALTER
+		 * SUBSCRIPTION that changed conflict_log_destination) after the
+		 * conflict was already reported to the server log by
+		 * ReportApplyConflict().  Nothing more to do; just discard the prepared
+		 * tuple and its context string.
+		 */
+		heap_freetuple(MyLogicalRepWorker->conflict_log_tuple);
+		MyLogicalRepWorker->conflict_log_tuple = NULL;
+		if (MyLogicalRepWorker->conflict_log_errcontext)
+		{
+			pfree(MyLogicalRepWorker->conflict_log_errcontext);
+			MyLogicalRepWorker->conflict_log_errcontext = NULL;
+		}
+	}
 
 	PopActiveSnapshot();
 	CommitTransactionCommand();
@@ -531,29 +547,65 @@ InitConflictIndexes(ResultRelInfo *relInfo)
  * GetConflictLogDestAndTable
  *
  * Fetches conflict logging metadata from the cached MySubscription pointer.
- * Sets the destination enum in *log_dest and, if applicable, opens and
- * returns the relation handle for the conflict log table.
+ * Reports whether conflicts should be logged to the table and/or the server
+ * log via *log_dest_table and *log_dest_logfile (either may be NULL if the
+ * caller is not interested), and, if a table is one of the destinations, opens
+ * and returns the relation handle for the conflict log table.
+ *
+ * The table is opened with try_table_open().  If the conflict log table has
+ * been dropped concurrently (e.g. by an ALTER SUBSCRIPTION that changed
+ * conflict_log_destination), NULL is returned and the reported destinations
+ * are adjusted to fall back to the server log (*log_dest_table = false,
+ * *log_dest_logfile = true), so the caller still records the conflict instead
+ * of failing.
  */
 Relation
-GetConflictLogDestAndTable(ConflictLogDest *log_dest)
+GetConflictLogDestAndTable(bool *log_dest_table, bool *log_dest_logfile)
 {
+	ConflictLogDest dest;
 	Oid			conflictlogrelid;
+	Relation	conflictlogrel;
 
 	/*
-	 * Convert the text log destination to the internal enum.  MySubscription
-	 * already contains the data from pg_subscription.
+	 * Convert the text log destination to the internal enum and report the
+	 * individual destinations to the caller.  MySubscription already contains
+	 * the data from pg_subscription.
 	 */
-	*log_dest = GetConflictLogDest(MySubscription->conflictlogdest);
+	dest = GetConflictLogDest(MySubscription->conflictlogdest);
+
+	if (log_dest_table)
+		*log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
+	if (log_dest_logfile)
+		*log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
 
 	/* Quick exit if a conflict log table was not requested. */
-	if (!CONFLICTS_LOGGED_TO_TABLE(*log_dest))
+	if (!CONFLICTS_LOGGED_TO_TABLE(dest))
 		return NULL;
 
 	conflictlogrelid = MySubscription->conflictlogrelid;
 
 	Assert(OidIsValid(conflictlogrelid));
 
-	return table_open(conflictlogrelid, RowExclusiveLock);
+	/*
+	 * Use try_table_open(): the table may have been dropped concurrently by an
+	 * ALTER SUBSCRIPTION that changed conflict_log_destination.
+	 */
+	conflictlogrel = try_table_open(conflictlogrelid, RowExclusiveLock);
+
+	/*
+	 * If the table is gone, fall back to logging the conflict to the server
+	 * log so it is not lost: report the table as no longer a destination and
+	 * force server-log reporting.
+	 */
+	if (conflictlogrel == NULL)
+	{
+		if (log_dest_table)
+			*log_dest_table = false;
+		if (log_dest_logfile)
+			*log_dest_logfile = true;
+	}
+
+	return conflictlogrel;
 }
 
 /*
diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h
index b727cc285e9..760f375d088 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -117,6 +117,7 @@ extern void ReportApplyConflict(EState *estate, ResultRelInfo *relinfo,
 								List *conflicttuples);
 extern void ProcessPendingConflictLogTuple(void);
 extern void InitConflictIndexes(ResultRelInfo *relInfo);
-extern Relation GetConflictLogDestAndTable(ConflictLogDest *log_dest);
+extern Relation GetConflictLogDestAndTable(bool *log_dest_table,
+										   bool *log_dest_logfile);
 extern void InsertConflictLogTuple(Relation conflictlogrel);
 #endif
-- 
2.54.0