Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Mon, Jun 22, 2020 at 6:38 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Jun 22, 2020 at 5:26 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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