MIME-Version: 1.0
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>
In-Reply-To: <8736bjoiax.fsf@ars-thinkpad>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Mon, 2 Mar 2020 11:56:33 +0530
Message-ID: 
 <CAA4eK1LuOGujY2v0u5bAp1VEKfWebHXBSEP0L-DO88p0MXFioQ@mail.gmail.com>
Subject: Re: ERROR: subtransaction logged without previous top-level txn
 record
To: Arseny Sher <a.sher@postgrespro.ru>
Cc: Andres Freund <andres@anarazel.de>, "Hsu, John" <hsuchen@amazon.com>,
	"pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Feb 9, 2020 at 9:37 PM Arseny Sher <a.sher@postgrespro.ru> wrote:
>
>
> Somehow I hadn't realized this earlier, so my comments/commit messages
> in patches above were not accurate here; I've edited them. Also in the
> first patch serialized snapshots are not no longer used for new slot
> creation at all, as Andres suggested above.
>

+ /*
+ * Don't use serialized snapshot if we are not sure where all
+ * currently running xacts will finish (new slot creation).
+ * (Actually, if we came here through xl_running_xacts, we could perform
+ * SNAPBUILD_FULL_SNAPSHOT -> SNAPBUILD_CONSISTENT transition properly,
+ * but added lines of code would hardly worth the benefit.)
+ */
+ if (builder->start_decoding_at == InvalidXLogRecPtr)
+ return false;

Instead of using start_decoding_at to decide whether to restore
snapshot or not, won't it be better to have new variable in SnapBuild
(say can_use_serialized_snap or something like that) and for this
purpose?

I think the patch is trying to use a variable that is not meant for
the purpose we are using for it, so not sure if it is the right
direction for the fix.

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