Re: Proposal: Conflict log history table for Logical Replication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: 2026-07-07 08:24:44
Message-ID: CAA4eK1LY2KOw9qxxBNkow9RygY+7u_dOzyzGCfzw2pWdbC7Jcw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 7, 2026 at 12:10 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Jul 2, 2026 at 4:09 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > I rebased the remaining patches on top of HEAD. So far, I have run
> > pgindent and completed the doc merge for 0001. The 0002 and 0003 are
> > just rebased.
>
> I just saw that a preparatory commit for this feature went in and it
> triggered me to look at this thread. What v63-0001 does is: when an
> error is thrown, the try/catch block sets aside the error, calls
> AbortOutOfAnyTransaction(), tries to insert the conflict tuple, and
> then re-throws the error.
>
> I think this is an entirely unacceptable design, for two reasons:
>
> 1. Setting aside the error in the middle of a try/catch block like
> this seems completely unsafe. What if there's another try/catch block
> on the stack that needs to run before AbortOutOfAnyTransaction
> fires()?
>

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.

If that's the concern, I don't think it applies here, because this
isn't a nested catch. The AbortOutOfAnyTransaction() and the deferred
insert run in start_apply()'s PG_CATCH (and ParallelApplyWorkerMain()
for the parallel worker), which is the outermost catch wrapping the
whole apply loop. There is no apply-level catch outside it. In fact,
AbortOutOfAnyTransaction() was already being called at this exact spot
before this patch; the patch only adds the deferred conflict-tuple
insert between that abort and the re-throw, so we are not aborting any
earlier than the code already did.

Having said that, if the worry is that this might not remain the
outermost catch in the future, we could convert this outer block to a
sigsetjmp-based handler, which would tie the abort unambiguously to
the top-level teardown and remove any dependence on its position in
the try/catch stack.

Please let me know if you had a different scenario in mind.

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

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-07-07 08:27:08 Re: Reject ill-formed range bounds histograms in pg_restore_attribute_stats()
Previous Message Ewan Young 2026-07-07 08:13:30 Reject ill-formed range bounds histograms in pg_restore_attribute_stats()