On Tue, Feb 11, 2020 at 10:02 AM Arseny Sher <a.sher@postgrespro.ru> wrote:
>
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
>
> >> 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.
>
> 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.
> making that potential restart_lsn real.
>
> I don't see how adding s2_checkpoint and s2_get_changes helps here. Do
> they really provoke error in your setup?
>
Yes, I am running each of the steps in test manually by using three
different terminals.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
