Re: Proposal: Conflict log history table for Logical Replication

On Fri, Jun 5, 2026 at 4:22 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Fri, Jun 5, 2026 at 3:06 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Fri, Jun 5, 2026 at 11:53 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Jun 4, 2026 at 4:05 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > >
> > > > I noticed that it is currently possible to acquire explicit locks on a CLT:
> > > >
> > > > --Session locks table and does not commit txn:
> > > > postgres=# BEGIN;
> > > > LOCK TABLE pg_conflict.pg_conflict_log_16481 IN SHARE MODE;
> > > > BEGIN
> > > > LOCK TABLE
> > > >
> > > > Doing so can cause the apply worker to block indefinitely when it
> > > > attempts to modify the CLT:
> > > >
> > > > [247433] LOG: logical replication apply worker for subscription
> > > > "sub1" has started
> > > > [247433] LOG: process 247433 still waiting for RowExclusiveLock on
> > > > relation 16482 of database 5 after 1001.030 ms
> > > > [247433] DETAIL: Process holding the lock: 245584. Wait queue: 247433.
> > > > [247433] CONTEXT: waiting for RowExclusiveLock on relation 16482 of database 5
> > > >
> > > > Toast Table behaviour:
> > > > postgres=*# LOCK TABLE pg_toast.pg_toast_16384 IN SHARE MODE;
> > > > ERROR: cannot lock relation "pg_toast_16384"
> > > > DETAIL: This operation is not supported for TOAST tables.
> > > >
> > > > Should we consider disallowing explicit LOCK TABLE operations on CLTs,
> > > > similar to how PostgreSQL handles TOAST tables? Or does anyone see any
> > > > legitimate use-case (I don't) where we would need to allow explicit
> > > > LOCKs on CLT?
> > >
> > > We need to add namespace-based checks here, as the current logic
> > > relies solely on relkind [1], which classifies TOAST tables
> > > separately. In my view, choosing to either allow or disallow this
> > > behavior will not cause significant inconvenience or seem unusual to
> > > anyone. Therefore, I prefer the path that minimizes special-purpose
> > > code. Since explicitly disallowing this requires additional
> > > special-purpose logic (as shown below [1]), allowing it seems to be
> > > the cleaner approach. Thoughts?
> >
> > Okay, upon analyzing this new logic, I too prefer to allow it.
> >
> > I was thinking if there is a way to set lock_timeout in
> > ProcessPendingConflictLogTuple() or try to acquire lock and if it
> > fails we hit 'ERRCODE_LOCK_NOT_AVAILABLE', log a different warning in
> > the log file and let the apply worker proceed.
> >
> > But if this too is complicated, I am fine with the current
> > implementation. Since LOCK TABLE is a well-known command, if a user
> > explicitly locks a CLT, they should be responsible for the
> > consequences such as blocking the apply worker.
>
> +1
>
> Here is the updated patch which fixes all open issues except Peter
> reported on 0004 patch, Vignesh would you take care of that?
>

Thank You Dilip.

v46-001:

1)

+static bool alter_sub_conflictlogdestination(Subscription *sub,
+ ConflictLogDest oldlogdest,
+ ConflictLogDest newlogdest,
+ Oid *conflicttablerelid);

+static void drop_sub_conflict_log_table(Oid subid, char *subname,
+

Can we name alter_sub_conflictlogdestination to
alter_sub_conflict_log_dest? Feel free to ignore if you find current
name better.

2)
Ran all the tests again on 0001 alone, inheritance is still working.
Let me know if we decided to retain it. IMO, it is better to block it
for the reasons stated earlier. The changes can be made in
MergeAttributes and ATExecAddInherit; we already have similar relation
based restrictions there, one more can be added for CLT.

~~

No major issue in 0001, it seems be in good shape. Will do one more
round of reveiw and testing on next version though.

thanks
Shveta