On Tue, Sep 22, 2020 at 5:18 PM Ajin Cherian <itsa...@gmail.com> wrote:
>
> On Sun, Sep 20, 2020 at 3:31 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> > 3.
> >
> > + /*
> > + * If it's ROLLBACK PREPARED then handle it via callbacks.
> > + */
> > + if (TransactionIdIsValid(xid) &&
> > + !SnapBuildXactNeedsSkip(ctx->snapshot_builder, buf->origptr) &&
> > + parsed->dbId == ctx->slot->data.database &&
> > + !FilterByOrigin(ctx, origin_id) &&
> > + ReorderBufferTxnIsPrepared(ctx->reorder, xid, parsed->twophase_gid))
> > + {
> > + ReorderBufferFinishPrepared(ctx->reorder, xid, buf->origptr, buf->endptr,
> > + commit_time, origin_id, origin_lsn,
> > + parsed->twophase_gid, false);
> > + return;
> > + }
> >
> >
> > I think we have already checked !SnapBuildXactNeedsSkip, parsed->dbId
> > == ctx->slot->data.database and !FilterByOrigin in DecodePrepare
> > so if those are not true then we wouldn't have prepared this
> > transaction i.e. ReorderBufferTxnIsPrepared will be false so why do we
> > need
> > to recheck this conditions.
>
> I didnt change this, as I am seeing cases where the Abort is getting
> called for transactions that needs to be skipped. I also see that the
> same check is there both in DecodePrepare and DecodeCommit.
> So, while the same transactions were not getting prepared or
> committed, it tries to get ROLLBACK PREPARED (as part of finish
> prepared handling). The check in if ReorderBufferTxnIsPrepared() is
> also not proper.
>If the transaction is prepared which you can ensure via ReorderBufferTxnIsPrepared() (considering you have a proper check in that function), it should not require skipping the transaction in Abort. One way it could happen is if you clean up the ReorderBufferTxn in Prepare which you were doing in earlier version of patch which I pointed out was wrong, if you have changed that then I don't know why it could fail, may be someplace else during prepare the patch is freeing it. Just check that. > I will need to relook > this logic again in a future patch. > No problem. I think you can handle the other comments and then we can come back to this and you might want to share the exact details of the test (may be a narrow down version of the original test) and I or someone else might be able to help you with that. -- With Regards, Amit Kapila.
