References: <AB5978B2-1772-4FEE-A245-74C91704ECB0@amazon.com>
 <87ftjifoql.fsf@ars-thinkpad>
 <20191024213157.7pm6niybfxgpvmgg@alap3.anarazel.de>
 <87eez1fh48.fsf@ars-thinkpad>
 <CAA4eK1L=MDbmGu5-+BmY7Svc07jr+ZabiH8C_qo3RSc8pgUpDQ@mail.gmail.com>
 <8736bs81sx.fsf@ars-thinkpad>
 <CAA4eK1LdNmrib1jub8b=KvYUrzXW0VT4P3MVPMyiMfMY3K64dA@mail.gmail.com>
 <871rrb942q.fsf@ars-thinkpad>
 <CAA4eK1LYzrZ_+8VhD_N_dsQwjxA9t+AyGKT-Wjnc8S7jCwAcBw@mail.gmail.com>
 <87zhdx76d5.fsf@ars-thinkpad>
 <CAA4eK1Jdh0zab=+D91MkFPbevzyjFCZsMwRsQxJh4F9+m_vCRA@mail.gmail.com>
 <CAA4eK1JnSKkNdgLAHWY+YCE_3Li454So_thyTyNJa_G3hSVscA@mail.gmail.com>
 <8736bjoiax.fsf@ars-thinkpad>
 <CAA4eK1LuOGujY2v0u5bAp1VEKfWebHXBSEP0L-DO88p0MXFioQ@mail.gmail.com>
 <87o8tf6w59.fsf@ars-thinkpad>
 <CAA4eK1JjGtRhgP6ShDrxQ+yU=nEQ5U+OG3Y4ZYME117qgK23AQ@mail.gmail.com>
User-agent: mu4e 1.2.0; emacs 26.0.50
From: Arseny Sher <a.sher@postgrespro.ru>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "Hsu\, John" <hsuchen@amazon.com>,
 "pgsql-bugs\@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: ERROR: subtransaction logged without previous top-level txn
 record
In-reply-to: 
 <CAA4eK1JjGtRhgP6ShDrxQ+yU=nEQ5U+OG3Y4ZYME117qgK23AQ@mail.gmail.com>
Date: Mon, 02 Mar 2020 16:37:04 +0300
Message-ID: <87mu8y7u9r.fsf@ars-thinkpad>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


Amit Kapila <amit.kapila16@gmail.com> writes:

> I think here you are trying to deduce the meaning.  I don't see that
> it can clearly define that don't use serialized snapshots. It is not
> clear to me why have you changed the below code, basically why it is
> okay to pass InvalidXLogRecPtr instead of restart_lsn?
>
> @@ -327,7 +327,7 @@ CreateInitDecodingContext(char *plugin,
>   ReplicationSlotMarkDirty();
>   ReplicationSlotSave();
>
> - ctx = StartupDecodingContext(NIL, restart_lsn, xmin_horizon,
> + ctx = StartupDecodingContext(NIL, InvalidXLogRecPtr, xmin_horizon,
>   need_full_snapshot, false,
>   read_page, prepare_write, do_write,
>   update_progress);

Because when we create the slot we don't demand to stream from some
specific point. In fact we just can't, because we don't know since which
LSN it is actually possible to stream, i.e. when we'd have good snapshot
and no old (which we haven't seen in full) xacts running. It is up to
snapbuild.c to define this point. The previous coding was meaningless:
we asked for some random restart_lsn and snapbuild.c would silently
advance it to earliest suitable LSN.

OTOH, when we are decoding from existing slot not only we know earliest
possible point, but to avoid missing xacts we must enforce streaming
since this very point despite the snapbuilder being unable (because he
might not know which xacts are running at point of the snapshot) to
check its safety.

start_decoding_at reflects the difference between these scenarios, and
serialized snapshots handling stems from here.

Thanks for looking into this.


-- cheers, arseny