Re: Proposal: Conflict log history table for Logical Replication

On Tue, Oct 27, 2026 at 12:20 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> 1)
> I checked and verified the fix posted by Hou-San on this idea. You’re
> right. With the fix, when the apply worker and the blocking command
> share the same lock_timeout, the user inititated TRUNCATE’s indefinite
> wait becomes a lock-timeout ERROR and the situation self-resolves (the
> TRUNCATE gives up and the worker proceeds). The only case that doesn’t
> self-resolve is when the apply worker has lock_timeout > 0 but the
> TRUNCATE session has 0; then the TRUNCATE hangs and the worker keeps
> erroring/retrying until it’s cancelled. But I think this configuration
> will be rare and impractical, so I don’t think this case is worth
> handling or documenting. Let me know if you think otherwise?. (I
> reproduced it by doing 'SET lock_timeout = 0' in session
> performingTRUNCATE but I guess there could be other ways as well to
> achieve this)
>

I agree with your conclusion and don't think we need any change for
this. We expect repeated ERROR in this rare scenario will let users
take action to proceed similar to PK violation error.

> 2)
> I did notice the message differs between the two lock_timeout
> settings. With lock_timeout > 0 we get the generic 'canceling
> statement due to lock timeout'; with 0 we get the more informative
> 'could not obtain lock on conflict log table + hint'. Do you think it
> would be better to maje the
> informative message appear in both by checking for a prepared
> transaction before the LockTimeout > 0 branch:
>
> if (MySubscription->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED &&
> LookupGXactBySubid(MySubscription->oid))
> ereport(ERROR, ... "could not obtain lock on conflict log table ...");
>
> if (MySubscription->twophasestate != LOGICALREP_TWOPHASE_STATE_ENABLED ||
> LockTimeout > 0)
> {
> LockRelationOid(relid, RowExclusiveLock);
> return;
> }
>

I don't see much value in adding more code to give a slightly better
ERROR message for a rare case.

> The fix otherwise looks good to me. One note: as with the DateStyle
> approach we dropped, this also temporarily sets a GUC (lock_timeout)
> in the apply worker, via the same save/restore mechanism. The
> difference is that the DateStyle one ran on the common path (every
> conflict logged to the table), whereas this only happens on a rare
> path; when two_phase is enabled, no prepared transaction exists, and
> the CLT lock still isn’t immediately available. Since it’s not on the
> hot path, I think it should be fine here, right?
>

It is not about the hot-code path but more about the need. In general,
we need to avoid setting the GUC mid-way in the code unless it is
necessary. I don't find the need for DateStyle stuff as necessary.

--
With Regards,
Amit Kapila.