Re: Proposal: Conflict log history table for Logical Replication

On Tue, Aug 4, 2026 at 12:06 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> >
> > The other options to avoid this form of error-recovery could be:
> >
> > 1. Do the abort/insert in ReportApplyConflict() (before raising the
> > error). For ERROR-level conflicts we could abort the current
> > transaction, insert the conflict row in a fresh transaction, and then
> > raise the ERROR — all from ReportApplyConflict(), where we're still in
> > normal execution. Aborting there should be fine as it's the apply
> > worker's own transaction. This avoids all the extra handling we are
> > doing in the catch block. See
> > v63-topup-0001-Insert-conflict-log-tuple-in-ReportApplyCo. OTOH, one
> > can argue that this is a special path for aborting a transaction
> > mid-way.
>
> In a hypothetical world where we support automatic conflict resolution
> in logical replication, we would not rely on raising an error in order
> to record the conflict information if the resolution method is
> 'last-writes-win' for example. I guess that the apply worker logs the
> conflict at LOG level and continues applying changes. In such a case,
> I think it might be better for the CLT feature not to insert a
> conflict information tuple based on the event where the execution
> raises an error like the topup patch does.
>

I see your point and agree that the day we have automatic resolution
strategy like last-write-win, we don't need the current mechanism of
preparetup-abort-newtrans-inserttup-errorout. The one more argument in
favor of your proposal for not inserting into CLT on ERROR-level
conflict is that if we do what we are doing now, we may end up adding
many-2 same rows into CLT as by default replication will continue from
same point and same ERROR happens again leading to same inserts in
CLT. This will add more maintenance burden in terms of cleaning up
such duplicate rows. But OTOH, what if the user manually removes the
conflicting row and the next cycle of apply is successful. Won't in
that case we will lose the required conflict information?

> That said, looking at the topop patch, I'm not sure it's okay to call
> AbortOutOfAnyTransaction() in ReportApplyConflict(). It aborts the
> transaction and cleans up all resources even though we still have
> several functions in the call stack. If some of the functions have
> PG_TRY()/PG_CATCH() in the future to clean up its resources, it would
> end up with a double-free problem.
>

This approach actually considers your point of automatic resolutions.
We want to do it at the place where we can decide how to handle
conflict based on the resolution strategy configured. If we do by
catching in the outermost caller then we will lose that opportunity.
The other idea is that we can think of changing APIs, so that for
ERROR cases, we return some value to outer layers and take the
decision in outer layer but not sure if this is anyway better. OTOH,
if we decide to follow your suggestion of not inserting into CLT then
this question won't arise in the first place though as I noted above,
that reintroduces the information-loss problem, so it's not a free
win.

> Also, I think the patch should test the case where multiple conflicts
> with different log levels happen in one transaction. I've not tested
> with the patch but I'm concerned that the inserting the LOG conflict
> information to CTl is also aborted altogether when raising the ERROR
> conflict.
>

Yes, this is true but the same would be true even without an ERROR
conflict, say, when an out-of-memory or any other ERROR happens during
the apply-transaction. Can we do any better for ERROR cases whether
conflict ERROR or any other ERROR?

--
With Regards,
Amit Kapila.