| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, 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>, 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-08-05 01:31:31 |
| Message-ID: | CAD21AoBQu1stUR0c2RS2eOTu5tg_LHABZp9BjV_X+zUnyvfVfA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Aug 4, 2026 at 3:53 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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?
My point was that we record them without keying the insertion to the
fact that an error was raised. So we would still capture the conflict
information you're concerned about losing.
I don't have a concrete idea yet, but ideally we would detect the
conflict before actually modifying the table and record it in a way
that doesn't rely on PG_TRY()/PG_CATCH(). Or we modify the table while
treating an ERROR-level conflict as a soft error, let the apply worker
abort the transaction, insert the conflict tuple in a fresh
transaction, and then re-throw the saved error. That way, the abort
would happen at a clean point in the apply worker during normal
execution, and the PG_CATCH block would no longer need to know
anything about conflict logging.
As for recording the same conflict repeatedly, we could add a
last_conflict_time column and simply update the timestamp when the
same conflict recurs, rather than inserting a new row each time.
Adding first_conflict_time and a counter alongside it might make the
intent clearer. We would need to define what makes two conflicts "the
same" and how we look up the existing row, so this needs more thought
too.
>
> > 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.
Yeah, it's a similar idea to the soft-error idea I mentioned above.
>
> > 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?
My understanding is that the conflict log table is meant to record the
same conflicts we have been writing to the server log. But the two
destinations don't fail in the same way. The log message is emitted
immediately and is unaffected by the fate of the apply transaction,
whereas a LOG-level conflict row is inserted in the apply transaction
and is rolled back if that transaction later fails but the same isn't
true for ERROR-level conflicts. So with conflict_log_destination =
'both', there are cases where a conflict appears in the server log but
not in the table, and never the other way around. That seems
surprising for an option whose purpose is to choose where the same
information goes. I'm not sure it's okay to discard many LOG-level
conflicts due to one ERROR-level conflict.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mark Atwood | 2026-08-05 01:33:25 | [PATCH v1 2/3] Fetch digests explicitly for cryptohash with OpenSSL 3.0 and later |
| Previous Message | jian he | 2026-08-05 01:27:11 | Re: MERGE/SPLIT PARTITIONS issues/questions |