On Tue, Jun 23, 2020 at 8:18 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 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.
I think you are right, during some intermediate code change, it crashed on that assert (I guess I might be adding invalidation to the sub-transaction but not sure what was that state) and I assumed that is the reason that I explained above but, now I see my assumption was wrong. I will put back that assert. By testing, I could not hit any case where we hit that assert even after my changes, still I will put more thought if by any chance our case is different then the base code. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
