Re: Proposal: Conflict log history table for Logical Replication

On Tue, 9 Jun 2026 at 9:18 AM, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:

> On Mon, Jun 8, 2026 at 7:01 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Mon, Jun 8, 2026 at 11:43 AM shveta malik <shveta(dot)malik(at)gmail(dot)com>
> wrote:
> > >
> > > 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.
> >
> > Yeah we may change that.
> >
> > > 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.
> >
> > After rethinking my previous stance on blocking these operations, let
> > me clarify the core principle I think we should follow for CLTs. I am
> > completely open to feedback on this approach:
> >
> > 1. Block Direct Mutations: We should block any operation that directly
> > modifies the CLT or its underlying data (e.g., DROP TABLE, ALTER
> > TABLE, INSERT, UPDATE), which impact the operation on CLT or update
> > the CLT data.
> > 2. Don't block Indirect/Edge-Case Operations: We should not write
> > custom code just to block edge cases that don't directly modify CLT
> > data or impact the operations on CLT. For example, if a user decides
> > to inherit from a CLT, that constitutes unexpected usage. We already
> > document (or can document) that dropping a subscription internally
> > drops the associated CLT. If a user inherits from it anyway and their
> > child tables are impacted when the subscription is dropped, that is
> > expected behavior and their usage issue.
>
> Fair enough. I'm okay with this approach, provided we document it
> clearly, perhaps as a CAUTION: users must be aware that DROP
> SUBSCRIPTION cascades the drop to the CLT and all its dependent
> objects, including any user-created inherited tables, view etc

Make sense..

>