Re: Proposal: Conflict log history table for Logical Replication

On Tue, Nov 3, 2026 at 3:17 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Nov 3, 2026 at 1:47 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > On Mon, Nov 2, 2026 at 3:17 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Sun, Nov 1, 2026 at 8:17 PM shihao zhong <zhong950419(at)gmail(dot)com> wrote:
> > > >
> > > > Thanks for addressing all of them.
> > > >
> > > > Lgtm
> > > >
> > >
> > > Thanks, here is a combined patch after fixing cosmetic issues
> > > including combining one test case with an existing test. I am planning
> > > to commit this tomorrow unless you or others have more comments on it.
> > > Sawada-san, the issue related to autovacuum cancellation during
> > > truncation raised by you was also addressed, please do let me know if
> > > you need more time to review?
> >
> > I've reviewed the v88 patch and have some comments and questions:
> >
> > In lock_conflictlog_relation(), LookupGXactBySubid() doesn't check
> > whether the prepared transaction actually holds a lock on the CLT, so
> > there are false positives. For example, when autovacuum is truncating
> > the CLT and the apply worker attempts to acquire a lock on it, the
> > apply worker raises an error even if the prepared transaction created
> > by the subscription doesn't hold any lock on the CLT. Since the check
> > is not precise, I think the subscription could error out easily. For
> > example, with disable_on_error = true, users would need to re-enable
> > the subscription frequently, which doesn't seem good.
>
> I agree with the side-effect of these false-positive cases. But adding
> lock-acquisition checks could complicate the code further.
>

Yes, I also thought about it previously and felt the same. To achieve
this we need to scan the lock table for holders on the relation and
match them to the subscription's prepared transactions (their dummy
PGPROCs). See attached top-up atop v88 generated by AI. I can
review/test it more if we decide to go with this strategy. I still
feel we shouldn't add this because such scenarios will be rare in
practice. It won't be difficult to address this if we really see such
field reports, so I would like to leave this code as is but if you
still think otherwise then we can take care with finding whether
prepared xact has locked CLT.

I think we
> can simply drop the fast-error block in lock_conflictlog_relation():
>
> if (LookupGXactBySubid(MySubscription->oid))
> ereport(ERROR, ... "could not obtain lock on conflict log table ..." ...);
>
> and let the worker fall through to the bounded wait (lock_timeout =
> deadlock_timeout + 5s) in all cases. Wouldn't that resolve the
> problem?
>
> IMO, this should eliminate the false positive where a blocking
> autovacuum is mistaken for a prepared transaction. A regular
> autovacuum should yield or be cancelled within that window, allowing
> the worker to acquire the lock and continue. A genuine prepared
> transaction, on the other hand, will continue holding the lock,
> causing only that case to reach the timeout and error. Thoughts?
>

Yeah, this sounds better than the above idea shared by me. But this
also won't be precise in telling the reason in case it is really
blocked by prepared xact, OTOH, maybe we don't need to care about the
same. Let us see what others think.

> > ---
> > Besides prepared transactions actually created by the apply worker,
> > IsTwoPhaseTransactionGidForSubid() checks only the GID format, so any
> > user can create a prepared transaction named pg_gid_<subid>_<xid>,
> > even an empty one. This makes the apply worker of a two_phase
> > subscription error out more easily, and it can also block DROP
> > SUBSCRIPTION and ALTER SUBSCRIPTION ... SET (conflict_log_destination
> > = 'log').
> >

Right but do we really need to worry about such cases? For example,
one can say the same about replication slots, say a user has created a
slot with name pg_xxx as generated internally by Create Subscription
then even Create Subscription can fail. I didn't notice such reports,
so I'm not sure if we should try to take care of this.

> > ---
> > ALTER SUBSCRIPTION ... OWNER TO is allowed even when the subscription
> > has prepared transactions. It waits for the lock held by the prepared
> > transaction, and the next conflict makes the apply worker error out.
> > Should we prohibit it as we do for DROP SUBSCRIPTION and changing
> > conflict_log_destination to 'log'?
> >

DROP SUBSCRIPTION and ALTER SUBSCRIPTION behaves differently because
they will prevent new workers from starting, so there won't be a
chance that a COMMIT PREPARED is applied and the dead lock is
released. But for the ALTER SUBSCRIPTION ... OWNER case, the same
issue won't happen, so it's more similar to the TRUNCATE command, and
we didn't added checks but we can add one if you think that is really
worth it. Again here there is a chance of false positive due to
unrelated Prepared Transactions unless we follow something like
attached nocfbot-v88-check_clt_locked_by_prepared_xact.

> > ---
> > Is there any way to carry over CLT data across major versions?
> >

Yes, there is a patch for this which is under discussion earlier, see
[1] but we thought we can deal with insertion as a pre-step and then
review that more care fully.

> > ---
> > The xid in local_conflicts is a 32-bit xid. I think it's better to
> > record a FullTransactionId instead. Otherwise, after xid wraparound,
> > two different local transactions can be recorded with the same xid,
> > which would easily confuse monitoring queries. We can't do the same
> > for remote_xid since the protocol sends only a 32-bit xid, but it
> > might be worth documenting.
> >

Agreed.

[1]: https://www.postgresql.org/message-id/CAD21AoBNTnF6Xzo-UqGxALNqyy85EFsPWDRuXGk0aibD2ugkgQ%40mail.gmail.com

--
With Regards,
Amit Kapila.