Re: Proposal: Conflict log history table for Logical Replication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(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 11:29:58
Message-ID: CAA4eK1L83+xwKgNeA0kNhOAG9rhAWb4v+88Kf84J921j0e1rqg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 5, 2026 at 7:02 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> 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.
>

IIUC, we are detecting the ERROR in a soft-error method only. See
ExecSimpleRelationInsert(), where if there a unique/primary key
violation, it will be returned as conflict and then we invoke
CheckAndReportConflict()->ReportApplyConflict() to report the
conflict. Now, problem with detecting before even inserting the tuple
into table is that it will require extra scans over index (there could
be multiple such scans as well if there is more than one index), also,
we may still need some way to ensure that the same tuple is not
inserted by someone else after our scan.

The current top-up 0002 patch handles it in ReportApplyConflict(). See
v66-0002-Perform-conflict-log-tuple-insertion-directly-in. It is not
clear to me if you want to instead handle it in an outer-layer, say at
the level of apply_handle_insert_internal, apply_handle_insert or
apply_dispatch or still at a layer above?

We should also consider the current change w.r.t future enhancements
where we want resolution strategies to be implemented with whatever
current design we choose. Say we consider three such strategies for
the unique_key violation, last_write_wins, keep_local, keep_remote.
last_write_wins can also have two outputs (a) local_wins which means
keep_local, (b) remote_wins which means keep local. The basic idea to
implement those would be:
- keep_local: could be implemented on the lines of ON CONFLICT DO
NOTHING (table_tuple_complete_speculative(rel, slot, false) ->
heap_abort_speculative()).
- keep_remote: could be implemented on the lines of ON CONFLICT DO
UPDATE aka back out the speculative tuple, then run a normal
heap_update/simple_table_tuple_update()-style update against the found
conflicting row using the remote's values, instead of leaving two rows
behind.
- last_write_wins = just a branch choosing which of the above two
(already-solved) paths to take, based on the ConflictTupleInfo.ts vs.
remote_commit_ts comparison from a couple of turns ago.

So invoking table_level APIs from so deep inside stack (from
conflict.c) doesn't sound to make sense. I feel the resolutions should
be from somewhere in worker.c. the other point is these strategies
will be MySubscription specific, so that also hints that worker.c
suits more.

BTW, attached find a POC which implements the desired mechanism in
worker.c in apply_handle_insert_internal/apply_handle_update_internal
where we are already handling LOG-level conflicts.

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

So, we need a separate key to search if the same tuple is present and
then do such an update. If so, this sounds reasonable but I feel we
can handle it as a separate patch/optimization?

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

I think the way to handle this would be to defer LOG-level inserts
too, accumulating them per-transaction and flushing them together with
the ERROR tuple in a fresh transaction after abort (or normally at
commit if no abort occurs). We can accumulate these conflict tuples in
an in-memory list at apply-transaction level and after a certain
threshold, we can spill the same way streaming apply already spills
large transactions' changes to a BufFile/FileSet rather than assuming
everything fits in memory. There could be some other ideas to handle
it but this is what occurred to me. What do you think?

--
With Regards,
Amit Kapila.

Attachment Content-Type Size
v66-0002-Perform-conflict-log-tuple-insertion-directly-in.patch application/octet-stream 17.0 KB
v66-0003-Return-unique-key-conflicts-to-the-apply-worker.patch application/octet-stream 14.4 KB
v66-0001-Implement-the-conflict-insertion-infrastructure-.patch application/octet-stream 94.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Xuneng Zhou 2026-08-05 11:39:02 Re: Streamify more code paths
Previous Message Shlok Kyal 2026-08-05 11:28:41 Re: Re-read subscription state after lock in AlterSubscription