Re: Proposal: Conflict log history table for Logical Replication

On Thu, Oct 29, 2026 at 2:21 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Wed, Oct 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Wed, Oct 28, 2026 at 2:28 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Oct 28, 2026 at 10:50 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > >
> > > > On Tue, Oct 27, 2026 at 5:03 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> > > > >
> > > > > Please find the updated patches v85. I also incorporated Shveta's doc
> > > > > suggestion[1] into the v85-0004 patch.
> > > >
> > > > Thanks for the patches. You did not post v85-0004, which you are
> > > > referring to in the above comment.
> > > >
> > > > I reviewed 001 mainly. A few comments:
> > > >
> > >
> > > Reviewed 002 and 003, one minor comment in 002, we have error messages
> > > like below:
> > >
> > > + errdetail("Dropping the conflict log table requires a lock that
> > > prepared transactions of this subscription may be holding."),
> > >
> > > + errdetail("Prepared transactions of the subscription may hold a lock
> > > on its conflict log table."),
> > >
> > > The second message uses the ambiguous "its". It would be clearer to
> > > explicitly refer to the 'subscription’s conflict log table', as in the
> > > first message.
> > >
> > > Suggestion:
> > > "Prepared transactions of the subscription may hold a lock on the
> > > subscription’s conflict log table."
> >
> > Attached patch set fixes all the comments of 0001 and 0002. It also
> > includes Nisha's offlist top up patch for 0001 and 0002, along with
> > some offlist suggestions from Amit. Additionally I have removed this
> > line "Renamed the conflict log table column remote_origin to
> > apply_origin." from the commit message, because this change is not on
> > top of the HEAD, it was just changed over previous version of the same
> > patch.
> >
>
> Just one trivial thing, in 002's new change in
> AlterSubscriptionOwner_internal(), it would be good to change comment
> slightly from 'while the prepared transaction waits for the apply
> worker' to 'while we wait for the prepared transaction to be resolved
> by the apply worker' for better clarity.

That makes sense; I will include this change in the next version, thanks.

--
Regards,
Dilip Kumar
Google