Re: Proposal: Conflict log history table for Logical Replication

On Tue, Nov 3, 2026 at 2:31 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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'm not sure it's that rare. Tuples copied by the initial table sync
always cause a conflict (update_origin_differs or
delete_origin_differs) when the apply worker modifies them for the
first time, if track_commit_timestamp is enabled. Also, in systems
using two-phase commit, it's common that the subscription has prepared
transactions. But I might be worrying too much.

TBH I'm still not sure why we really need the special behavior of
erroring out only for the CLT. I agree that it's an internal behavior
and users might not be aware of it, but I think users who don't know
it would be equally confused when the apply worker errors out because
of it.

>
> > > ---
> > > 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.

I think it's a bit different from the replication slot case. Creating
a replication slot requires the REPLICATION privilege, which is a high
privilege. On the other hand, any user can create a prepared
transaction.

>
> > > ---
> > > 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.

Agreed.

>
> > > ---
> > > 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.

Nice. Thank you for letting me know.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com