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

On Wed, Feb 12, 2020 at 1:42 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Feb 12, 2020 at 9:09 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Feb 12, 2020 at 8:46 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Tue, Feb 11, 2020 at 5:36 PM Arseny Sher <a.sher@postgrespro.ru> wrote:
> > > >
> > > >
> > > > Amit Kapila <amit.kapila16@gmail.com> writes:
> > > >
> > > > >> That's weird, it reliably fails with expected error for me. There are
> > > > >> already two s2_checkpoint's: first establishes potential (broken)
> > > > >> restart_lsn (serializes snapshot after first xl_xact_assignment of s0
> > > > >> xact, but before first record of s1 xact), the second ensures
> > > > >> s2_get_changes directly following it will actually advance the slot,
> > > > >>
> > > > >
> > > > > In my case, s2_get_changes doesn't seem to be advancing the restart
> > > > > lsn because when it processed running_xact by s2_checkpoint, the slots
> > > > > confirm flush location (slot->data.confirmed_flush) was behind it.  As
> > > > > confirmed_flush was behind running_xact of s2_checkpoint, it couldn't
> > > > > update slot->candidate_restart_lsn (in function
> > > > > LogicalIncreaseRestartDecodingForSlot).  I think the confirmed_flush
> > > > > location will only be updated at the end of get_changes.  This is the
> > > > > reason I need extra get_changes call to generate an error.
> > > > >
> > > > > I will think and investigate this more, but thought of sharing the
> > > > > current situation with you.  There is something different going on in
> > > > > my system or maybe the nature of test is like that.
> > > >
> > > > Ah, I think I know what's happening -- you have one more
> > > > xl_running_xacts which catches the advancement -- similar issue is
> > > > explained in the comment in oldest_xmin.spec.
> > > >
> >
> > Right, that is why in my case get_changes were required twice.  After
> > calling get_changes as we do in oldest_xmin.spec will make test case
> > reliable.
> >
>
> Attached is a patch where I have modified the comments and slightly
> edited the commit message.  This patch was not getting applied in v11
> and branches lower than that, so I prepared a patch for those branches
> as well. I have tested this patch till 9.5 and it works as intended.
>
> Can you also once check the patch and verify it in back-branches?

I have checked the patch and it looks fine to me.  I have also tested
it on the back branches and it works fine.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com