From e5954eb1ef36a98b7dcc31697dd2427070892de3 Mon Sep 17 00:00:00 2001 From: Amit Kapila 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