Re: Proposal: Conflict log history table for Logical Replication

On Wed, Nov 4, 2026 at 3:59 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Nov 4, 2026 at 03:08 PM Peter Smith wrote:
> >
> > I think the GUC timezone is yet another setting that can affect how a
> > key value is written, so it should be similary mentioned.
>
> I confirmed that you are right, changing the subscriber's timezone
> setting changes the UTC offset in the recorded string:
>
> -- Conflict 1 recorded with timezone = 'UTC',
> -- Conflict 2 recorded with timezone = 'Asia/Tokyo' on the same key:
>
> SELECT replica_identity,
> replica_identity->>'ts' = '2025-01-01 00:00:00+00' AS text_match,
> (replica_identity->>'ts')::timestamptz = '2025-01-01
> 00:00:00+00' AS cast_match
> FROM pg_conflict.pg_conflict_log_16390;
>
> replica_identity | text_match | cast_match
> ---------------------------------+------------+------------
> {"ts":"2025-01-01 00:00:00+00"} | t | t
> {"ts":"2025-01-01 09:00:00+09"} | f | t
>
> I have added timezone to the list in this version.
>
> ~
>
> Here is the v91 patch set, which updates the xid field in
> local_conflicts to store the 64-bit full transaction ID (xid8).
>
> Rather than converting the 32-bit XID to a FullTransactionId later when
> building the conflict log tuple, the patch converts it when the
> conflicting tuple is identified, while its buffer is still pinned.
>
> This is necessary for update_deleted: because the dead tuple is not
> locked and its buffer pin is released as soon as the scan in finishes,
> converting the XID later (if retention is concurrently stopped due to
> max_retention_duration) could allow VACUUM to remove the dead tuple and
> advance oldestXid past delete_xid, resulting in an incorrect epoch. For
> other conflicts (such as update_origin_differs), the tuple remains
> locked and pinned through conflict reporting, so this race cannot occur,
> but we convert the XID at the conflict detection sites as well to keep
> the handling consistent.
>
> The doc has also been updated to note that local_conflicts[].xid is of
> type xid8 and that remote_xid can wrap around on the publisher. I didn't
> add any new tests, as I couldn't find existing tests that specifically
> cover the full XID, so adding one doesn't seem worth it here.
>

The xid fix looks okay to me. One minor point: in
CheckAndReportConflict(), the epoch-based conversion

conflicttuple->xmin =
FullTransactionIdFromAllowableAt(ReadNextFullTransactionId(), xmin);

isn’t strictly needed. These are ERROR-level conflicts, which are
never written to the conflict log table, so conflicttuple->xmin is
only consumed by the server-log in ReportApplyConflict() after
truncating back to the 32-bit xid (epoch thrown). But I think, it is
good to still retain it as-is for consistency with the other
conflict-detection sites.

~~

I am not sure if you have started taking comments on 002 yet. Here are
a few comments; feel free to defer them to later versions if we want
to conclude on 001 first.

1)
CONFLICT_LOG_RELATION_NAME_FMT is only used in backend code
(conflict.c and subscriptioncmds.c), so I think it need not be inside
the '#ifdef EXPOSE_TO_CLIENT_CODE'?

2)
+ if (dopt->binary_upgrade)
+ tbinfo->dobj.dump = DUMP_COMPONENT_DEFINITION | DUMP_COMPONENT_ACL;
+ else
+ tbinfo->dobj.dump = DUMP_COMPONENT_NONE;

In a regular (non-binary-upgrade) dump, we do not dump definition of
CLT (which is fine) as the table gets created if needed on dumped
'ALTER SUB' but we do not even dump privileges granted on it (because
the name of CLT is not known). So post-upgrade user has to execute
GRANTs etc again (if any). I think it would be worth documenting this
limitation in the pg_dump doc, alongside the existing information
about subscriptions.

3)
dumpTableSchema:
+ bool conflict_log_table =
+ (strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0);

selectDumpableNamespace:
+ if (strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0)

Should we make a function for this usage:

static bool
is_conflict_log_table(const TableInfo *tbinfo)
{
return strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") == 0;
}

thanks
Shveta