Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

From Dilip Kumar
Subject Re: Proposal: Conflict log history table for Logical Replication
Date
Msg-id CAFiTN-tnBhCU8opWAjp1xYkjqGeXtQ1tfDZHbYBS7TifymXEvw@mail.gmail.com
Whole thread
In response to Re: Proposal: Conflict log history table for Logical Replication  (Nisha Moond <nisha.moond412@gmail.com>)
Responses Re: Proposal: Conflict log history table for Logical Replication
Re: Proposal: Conflict log history table for Logical Replication
List pgsql-hackers
On Mon, Jun 22, 2026 at 4:32 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Sun, Jun 21, 2026 at 7:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Jun 18, 2026 at 9:33 AM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > On Tue, Jun 16, 2026 at 6:54 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > IMHO we should just log WARNING and continue the apply worker on
> > > > conflict insertion failure, lets see what other thinks on this.
> > > >
> > >
> > > I have the same opinion. Allowing CLT to block the apply worker would
> > > be undesirable; CLT is a history/logs collection feature and should
> > > not interrupt core logical replication work.
> > >
> >
> > I think the insert can fail in rare cases like disk getting full while
> > writing WAL or some internal memory ERROR and the ERROR could be
> > persistent which means the LOG will be filled with the same WARNING if
> > there are many conflicts. Also, users may not like missing out on
> > conflict information. So, we can ERROR out and let users fix the
> > situation. Additionally, the nested try-catch to downgrade ERROR to
> > WARNING also looks ugly and a source of future bugs and maintenance
> > burden. The attached patch tries to fix this by ERRORing out on
> > insertion failure and attaching the required conflict info as a
> > context of ERROR. The patch also improved the ReportApplyConflict()
> > non-ERROR paths by displaying the conflict information in server LOGs
> > before inserting the same into CLT so that if insertion fails, the
> > complete conflict info can be present in server LOGs. See
> > v52-1-amit.Improve-error-handling-for-conflict-log-table-ins.
> >
> > Additionally, there is another problem with 0003 where when a parallel
> > apply worker hits an ERROR-level conflict it logs the conflict to the
> > conflict log table in a new transaction in its error path, after
> > aborting the failed apply transaction.  But the leader detects worker
> > failure in pa_wait_for_xact_finish() by waiting on the worker's
> > transaction lock, and AbortOutOfAnyTransaction() releases that lock:
> > the leader unblocks, sees a non-finished state, raises "lost
> > connection to the logical replication parallel apply worker", and
> > tears the worker down -- which can SIGTERM it mid-insert and lose the
> > conflict log row, besides being a misleading message. The attached
> > top-up patch v52-2-amit.fix_parallel_apply_logging fixes that by
> > introducing PARALLEL_TRANS_ERROR state.
>
> I reproduced the above issue and verified the fix for it in
> v52-2-amit.fix_parallel_apply_logging. Here is a TAP test for the
> same.
> The attached top-up patch applies on top of the latest v53-0005 patch.
>
I have merged Amit's patch and Nisha's tap test patch and tested all
the cases discussed, and those are working fine.

--
Regards,
Dilip Kumar
Google

Attachment

pgsql-hackers by date:

Previous
From: Marcos Pegoraro
Date:
Subject: Re: [PATCH] Add pg_get_table_ddl() to reconstruct CREATE TABLE statements
Next
From: Pavel Stehule
Date:
Subject: Re: Global temporary tables