Re: Proposal: Conflict log history table for Logical Replication

On Thu, Jul 9, 2026 at 7:40 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> 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.
>

Right.

> 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.
>

The parallel apply worker is writing to the conflict table and or to
log but in addition to that it sends the error_info to the leader
worker and that part is required even without conflict logging.

The reason is that an ERROR-level conflict (say insert_exists) is a
transaction failure, and in parallel apply, the leader has to know
about a worker's failure. The mechanism pre-exists this patch: to
maintain commit order, the leader - on receiving a transaction's
commit from upstream - waits for the parallel apply worker to finish
that transaction before proceeding with the pending apply actions; and
if a parallel worker exits with an ERROR, the leader tears down all
parallel workers and restarts replication from the last successfully
applied transaction. So the worker should propagate its error to the
leader regardless of conflict logging.

The only thing this patch adds on top is ordering: the worker must
finish inserting into the conflict log table before that error is
reported to the leader. Otherwise, once AbortOutOfAnyTransaction()
releases the transaction lock the leader is waiting on, the leader
could tear this worker down while it is still writing the conflict
row. That is why the worker sets the error state before aborting and
re-throws (which is what reports the error to the leader) only after
the insert has completed.

--
With Regards,
Amit Kapila.