| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
| Cc: | Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(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-10-15 04:17:28 |
| Message-ID: | CAJpy0uC=ZbZrzUzZoh3cpcBJEFR6MROD61wOGXGUjuJ9Xakapw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Oct 14, 2026 at 8:16 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
>
> 1) Does the remote_origin column in CLT add any value?
> + if (replorigin_xact_state.origin != InvalidReplOriginId)
> + replorigin_by_oid(replorigin_xact_state.origin, true, &remote_origin);
>
> IIUC, replorigin_xact_state.origin is the subscription's own origin,
> set at worker start, so every row has pg_<subid>.
It could also be pg_<subid>_<relid>, for a tablesync worker. But
again, it is not very useful as both subid and relid can be figured
out from CLT table itself. So I too currently do not see any use of
this column, unless I am missing some case?
> A CLT only holds
> conflicts of its own subscription, and its name already contains the
> subid. Once the subscription is dropped, the CLT goes with it, and
> even a saved copy can't be traced back to a publisher node. Should we
> drop this column? Thoughts?
>
> 2) conflict.c: insert_conflict_log_tuple():
> + heap_insert(conflictlogrel, tuple,
> + GetCurrentCommandId(true), HEAP_INSERT_NO_LOGICAL, NULL);
> Should we add a comment explaining why HEAP_INSERT_NO_LOGICAL is still
> needed even though a CLT can't be added to a publication?
>
> 3) v78-0004 removed "or when replica identity is not applicable" from
> the replica_identity description, since searchslot can never be NULL.
> I think v80 added it back:
> + when replica identity is not applicable.</entry>
> The code also has Assert(!TupIsNull(searchslot)), so replica_identity
> is NULL only when replica_identity_full is true. Should remove this
> part again.
>
> 4) The conflict below is from a 2PC txn hitting delete_missing while
> the worker processes PREPARE TRANSACTION:
>
> relname | conflict_type | remote_xid | remote_commit_lsn | remote_commit_ts
> ---------+----------------+------------+-------------------+------------------
> t2pc | delete_missing | 669 | 0/017D0958 |
>
> Using the prepare LSN as remote_commit_lsn for 2PC seems intentional.
> But remote_commit_ts stays NULL, because the commit time isn't known
> during PREPARE TRANSACTION. Could we use the prepare time here, as we
> do with the prepare lsn? It may not help with resolution, though. If
> not, we should document the NULL for 2PC too; currently the docs
> mention it only for the parallel apply worker.
>
> 5) The text recorded for a key depends on the apply worker's output
> settings. With bytea_output = escape or quote_all_identifiers = on set
> on the subscriber, the same key is stored differently from the
> default. For example, the two settings were toggled between two
> conflicts on the same keys (tb has a bytea key, tr a regclass[] key):
>
> postgres=# SELECT relname, conflict_type, replica_identity::text FROM
> pg_conflict.pg_conflict_log_16398;
> relname | conflict_type | replica_identity
> ---------+----------------+--------------------------------
> -- bytea_output
> tb | delete_missing | {"k":"\\x80ff"} -- hex case
> tb | delete_missing | {"k":"\\200\\377"} -- escape case
>
> --quote_all_identifiers
> tr | delete_missing | {"k":"{pg_class}"} -- off case
> tr | delete_missing | {"k":"{\"\\\"pg_class\\\"\"}"} -- on case
>
> The values themselves are correct: casting back to the column type
> gives the same key, e.g. (replica_identity->>'k')::bytea or
> ::regclass[]. But anyone searching or grouping conflicts by
> replica_identity->>'k' as text gets split or missing results. The
> current docs say the value is "the same text a SELECT of that column
> would show", which only holds when the reader's settings match the
> apply worker's. It would help to document this.
Doc looks okay to me.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Koshi Shibagaki (Fujitsu) | 2026-10-15 04:54:43 | Re: parallel data loading for pgbench -i |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-10-15 03:54:43 | RE: Patch for migration of the pg_commit_ts directory |