Re: ERROR: subtransaction logged without previous top-level txn record

On Mon, Feb 10, 2020 at 6:34 PM Arseny Sher <a.sher@postgrespro.ru> wrote:
>
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
>
> >> I don't believe you can that without persisting additional
> >> data. Basically, what we need is list of transactions who are running at
> >> the point of snapshot serialization *and* already wrote something before
> >> it -- those we hadn't seen in full and can't decode. We have no such
> >> data currently. The closest thing we have is xl_running_xacts->nextXid,
> >> but
> >>
> >>   1) issued xid doesn't necessarily means xact actually wrote something,
> >>      so we can't just skip xl_xact_assignment for xid < nextXid, it might
> >>      still be decoded
> >>   2) snapshot might be serialized not at xl_running_xacts anyway
> >>
> >> Surely this thing doesn't deserve changing persisted data format.
> >>
> >
> > I agree that it won't be a good idea to change the persisted data
> > format, especially in back-branches.  I don't see any fix which can
> > avoid this without doing major changes in the code.  Apart from this,
> > we have to come up with a solution for point (3) discussed in the
> > above email [1] which again could be change in design.  I think we can
> > first try to proceed with the patch
> > 0002-Stop-demanding-that-top-xact-must-be-seen-before--v2 and then we
> > can discuss the other patch.  I can't see a way to write a test case
> > for this, can you think of any way?
>
> Yeah, let's finally get it.
>
> Attached is raw version of isolation test triggering false
> 'subtransaction logged without...' (case (1)).
>

This didn't reproduce the desired error for me (tried without a
patch).  I think you need to add two more steps ("s2_checkpoint"
"s2_get_changes") at the end of the test to set the restart_lsn at the
appropriate location.

> However, frankly I don't
> see much value in it, so I'm dubious whether it should be included in
> the patch.
>

I think this will surely test some part of the system which was not
tested before, mainly having some subxacts without top-xact getting
decoded even though we don't need to send such a transaction.  Can you
prepare a complete patch (for
Stop-demanding-that-top-xact-must-be-seen-before-sub) having this test
as part of it?

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