Re: Proposal: Conflict log history table for Logical Replication

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?

--
Regards,
Dilip Kumar
Google