On Wed, Nov 26, 2025 at 5:59 AM Masahiko Sawada <[email protected]> wrote:
>
> On Sun, Nov 16, 2025 at 10:40 PM Amit Kapila <[email protected]> wrote:
> >
> > On Fri, Nov 14, 2025 at 3:14 PM Masahiko Sawada <[email protected]> 
> > wrote:
> > >
> > > On Fri, Nov 14, 2025 at 1:25 AM shveta malik <[email protected]> 
> > > wrote:
> > > >
> > > > In an offline discussion with Kuroda-san, we realized that TRUNCATE
> > > > may hit Assert(XLogLogicalInfoActive()) in ExecuteTruncateGuts() under
> > > > our current implementation, where logical decoding is disabled lazily.
> > > >
> > > > Consider the case where there’s only one logical slot and we attempt
> > > > to drop it. The backend issues the drop request, but before the
> > > > checkpointer actually disables logical decoding, a TRUNCATE is
> > > > executed. Since logical decoding is still marked as active at that
> > > > moment, the ExecuteTruncate() appends the OID to relids_logged.
> > > > However, by the time control reaches ExecuteTruncateGuts, the
> > > > checkpointer has already disabled logical decoding resulting in
> > > > Assert.
> > > >
> > > > TRAP: failed Assert("XLogLogicalInfoActive()"), File: "tablecmds.c",
> > >
> > > Good find. I think this assertion is no longer valid given that
> > > XLogLogicalInfoActive() can change dynamically. It's safe to write
> > > logica information to WAL records even if it's not strictly required,
> > > so we can keep writing logical info even if XLogLogicalInfoActive()
> > > comes to return false in the middle. In the opposite case where
> > > logical decoding becomes enabled in the middle, a similar thing
> > > happens but we will wait for such a transaction to finish before
> > > starting the logical decoding. Therefore, we can remove the assertion.
> > > What do you think?
> > >
> >
> > I also think it is safe to remove this assertion.
> >
> > > I'll check if there are other similar issues due to this patch.
> > >
> >
> > Yes, that makes sense. In the worst case, if we find some hard to
> > remove dependencies either now or in future then we can let DISABLE of
> > logical_decoding operation wait for current transactions to finish
> > like we are doing for corresponding ENABLE operation.
>
> After thinking of this case, I'm concerned that we would hit the
> existing similar assertion failures or assertions that future changes
> could introduce because the value returned by XLogLogicalInfoActive()
> could be changed even within the same transaction whereas the result
> from XLogStandbyInfoActive() doesn't. One possible change would be to
> ensure that XLogLogicalInfoActive() returns the same result within the
> same transaction. It would be more straightforward and closer to the
> current GUC-based behavior.
>

Agreed that this is worth considering. So, the only downside it could
have is the performance impact where it is not used like when
wal_level is replica and the user didn't create any logical slot. So,
short transactions where we do call XLogLogicalInfoActive() directly
or indirectly only once or twice, we always need to invoke
CheckXLogLogicalInfoSlow(). Can we try some workloads to see if there
is any noticeable impact?

-- 
With Regards,
Amit Kapila.


Reply via email to