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>
 <CAA4eK1Kcsib6UG7zFPrL-h2fvnByfKWxq8Xvzg=2hxUebYwt=g@mail.gmail.com>
User-agent: mu4e 1.1.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: 
 <CAA4eK1Kcsib6UG7zFPrL-h2fvnByfKWxq8Xvzg=2hxUebYwt=g@mail.gmail.com>
Date: Mon, 10 Feb 2020 16:04:46 +0300
Message-ID: <87wo8ulhjl.fsf@ars-thinkpad>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Precedence: bulk

--=-=-=
Content-Type: text/plain


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)). However, frankly I don't
see much value in it, so I'm dubious whether it should be included in
the patch.


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline; filename=subxact_logged_without_top_test.patch

diff --git a/contrib/test_decoding/specs/subxact_logged_without_top.spec b/contrib/test_decoding/specs/subxact_logged_without_top.spec
new file mode 100644
index 0000000000..55b51357a9
--- /dev/null
+++ b/contrib/test_decoding/specs/subxact_logged_without_top.spec
@@ -0,0 +1,51 @@
+
+setup
+{
+    SELECT 'init' FROM pg_create_logical_replication_slot('isolation_slot', 'test_decoding'); -- must be first write in xact
+    CREATE TABLE harvest(apples integer);
+    CREATE OR REPLACE FUNCTION subxacts() returns void as $$
+    BEGIN
+      FOR i in 1 .. 65 LOOP
+        BEGIN
+          INSERT INTO harvest VALUES (42);
+        EXCEPTION
+        WHEN OTHERS THEN
+	  RAISE;
+        END;
+    END LOOP;
+    END; $$LANGUAGE 'plpgsql';
+}
+
+teardown
+{
+    DROP TABLE IF EXISTS harvest;
+    SELECT 'stop' FROM pg_drop_replication_slot('isolation_slot');
+}
+
+session "s0"
+setup { SET synchronous_commit=on; }
+step "s0_begin" { BEGIN; }
+step "s0_first_subxact" {
+    DO LANGUAGE plpgsql $$
+      BEGIN
+        BEGIN
+          INSERT INTO harvest VALUES (41);
+	EXCEPTION WHEN OTHERS THEN RAISE;
+	END;
+      END $$;
+}
+step "s0_many_subxacts" { select subxacts(); }
+step "s0_commit" { COMMIT; }
+
+session "s1"
+setup { SET synchronous_commit=on; }
+step "s1_begin" { BEGIN; }
+step "s1_dml" { INSERT INTO harvest VALUES (43); }
+step "s1_commit" { COMMIT; }
+
+session "s2"
+setup { SET synchronous_commit=on; }
+step "s2_checkpoint" { CHECKPOINT; }
+step "s2_get_changes" { SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); }
+
+permutation "s0_begin" "s0_first_subxact" "s2_checkpoint" "s1_begin" "s1_dml" "s0_many_subxacts" "s0_commit" "s2_checkpoint" "s2_get_changes" "s1_commit" "s2_get_changes"

--=-=-=
Content-Type: text/plain


-- cheers, Arseny

--=-=-=--