Re: Proposal: Conflict log history table for Logical Replication

On Tue, Jul 7, 2026 at 4:24 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> If I understand you correctly, the concern is that
> AbortOutOfAnyTransaction() tears down the entire transaction stack, so
> if it runs from a nested PG_CATCH, any outer catch block still queued
> on the stack that expects a live (sub)transaction would then execute
> against already-aborted state.

The concern is about this code:

+ PG_TRY();
+ {
+ LogicalParallelApplyLoop(mqh);
+ }
+ PG_CATCH();
+ {
+ MemoryContext oldcontext;
+ ErrorData *edata;
+
+ /*
+ * Reset the origin state to prevent the advancement of origin
+ * progress if we fail to apply. Otherwise, this will result in
+ * transaction loss as that transaction won't be sent again by the
+ * server.
+ */
+ replorigin_xact_clear(true);
+
+ /*
+ * Copy the error and recover to an idle state so we can insert the
+ * deferred conflict log tuple (if any) before re-throwing. Copy the
+ * error into a longer-lived context first, as it may have been raised
+ * under ErrorContext. Also reset the error context stack: the
+ * callbacks in effect when the error was thrown belong to unwound
+ * stack frames, and the deferred insert installs its own.
+ */
+ oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+ edata = CopyErrorData();
+ MemoryContextSwitchTo(oldcontext);
+
+ FlushErrorState();
+ error_context_stack = NULL;
+
+ /*
+ * Tell the leader we failed and are about to report the error and log
+ * the conflict. This must be set before AbortOutOfAnyTransaction()
+ * below releases the transaction lock that the leader waits on in
+ * pa_wait_for_xact_finish(); otherwise the leader would see a
+ * non-finished state, assume the connection was lost, and tear this
+ * worker down while it is still writing the conflict log tuple.
+ */
+ pa_set_xact_state(MyParallelShared, PARALLEL_TRANS_ERROR);
+
+ AbortOutOfAnyTransaction();
+
+ /*
+ * Insert the deferred conflict log tuple before re-throwing.
+ * Re-throwing is what reports the error to the leader (via the error
+ * queue set up above), so the insertion must happen first: otherwise
+ * the leader could start tearing down this worker while it is still
+ * writing the conflict log tuple. If the insertion itself fails,
+ * that error (annotated with the conflict context, see
+ * InsertConflictLogTuple) propagates to the leader instead of the
+ * original.
+ */
+ ProcessPendingConflictLogTuple();
+
+ /* Re-throw the original error, which reports it to the leader. */
+ ReThrowError(edata);
+ }
+ PG_END_TRY();

I see your point about this being the outermost try/catch, but this
still doesn't look good to me. It seems different than the way other
workers do error recovery, and I am really doubtful that is is safe.

> > 2. If inserting into the conflict table itself errors out, then the
> > conflict log table will be permanently out-of-sync with the status of
> > the replicated table.
>
> The conflict log table can't permanently diverge from the replicated
> table. On the error path we call replorigin_xact_clear() before
> inserting the conflict row, so the CLT-insert transaction commits
> without advancing the replication origin. The origin therefore stays
> at the last-confirmed LSN, and the errored transaction is re-streamed
> and re-applied on the next attempt; which also re-attempts the
> conflict logging. If the CLT insert itself fails, nothing is committed
> and the same replay re-does both. Is there a specific path where you
> see the origin advancing without the row being applied that could lead
> to divergence?

I'm glad we agree on the goal. If an error inserting into the conflict
table doesn't advance the replication position, then we should
eventually either (a) succeed in replicating the data or (b) succeed
in inserting the error, even if it takes many retries before one or
the other happens. But why are we trying to send the error to the
leader here? It seems to me that a replication apply worker should be
directly sending the errors to either the log or the conflict table,
and the leader shouldn't need to know about it.

--
Robert Haas
EDB: http://www.enterprisedb.com