On Mon, Jun 22, 2020 at 6:38 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Mon, Jun 22, 2020 at 5:26 PM Amit Kapila <amit.kapil...@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