Re: Proposal: Conflict log history table for Logical Replication

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]
> @@ -92,6 +93,14 @@ RangeVarCallbackForLockTable(const RangeVar *rv,
> Oid relid, Oid oldrelid,
>
> rv->relname),
>
> errdetail_relkind_not_supported(relkind)));
>
>
>
> + /* Disallow explicit LOCK TABLE on conflict log tables */
> + if (IsConflictLogTableNamespace(get_rel_namespace(relid)))
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot lock relation \"%s\"",
> + rv->relname),
> + errdetail("This operation is not
> supported for conflict log tables.")));
> +
>
> /*
> * Make note if a temporary relation has been accessed in this
> * transaction.
> @@ -198,6 +207,14 @@ LockViewRecurse_walker(Node *node,
> LockViewRecurse_context *context)
> relkind != RELKIND_VIEW)
> continue;
>
>
> + /* Disallow locking conflict log tables even
> via views. */
> + if
> (IsConflictLogTableNamespace(get_rel_namespace(relid)))
> + ereport(ERROR,
> +
> (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot lock
> relation \"%s\"",
> + relname),
> + errdetail("This
> operation is not supported for conflict log tables.")));
> +
>
>
>
> --
> Regards,
> Dilip Kumar
> Google