Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions

On Mon, Jun 22, 2020 at 6:38 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Jun 22, 2020 at 5:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > > > @@ -2012,8 +2014,6 @@ ReorderBufferForget(ReorderBuffer *rb,
> > > > TransactionId xid, XLogRecPtr lsn)
> > > >   if (txn->base_snapshot != NULL && txn->ninvalidations > 0)
> > > >   ReorderBufferImmediateInvalidation(rb, txn->ninvalidations,
> > > >      txn->invalidations);
> > > > - else
> > > > - Assert(txn->ninvalidations == 0);
> > > >
> > > > Why this Assert is removed?
> > >
> > > Even if the base_snapshot is NULL, now we are collecting the
> > > txn->invalidation.
> > >
> >
> > But there doesn't seem to be any check even before this patch which
> > directly prohibits accumulating invalidations in DecodeCommit.  We
> > have check for base_snapshot in ReorderBufferCommit.  Did you get any
> > failure with that check?
>
> Because earlier ReorderBufferForget for toptxn will be called if the
> top transaction is aborted and in abort case, we are not logging any
> invalidation so that will be 0.  However same is not true now.
>

AFAICS, ReorderBufferForget() is called (via DecodeCommit) only when
we need to skip the transaction.  It doesn't seem to be called from
Abort path (DecodeAbort/ReorderBufferAbort doesn't use
ReorderBufferForget).  I am not sure which code path are you referring
here, can you please share the code flow which you are referring to
here.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com