From: Arseny Sher <a.sher@postgrespro.ru>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Hsu\, John <hsuchen@amazon.com>
Cc: pgsql-bugs\@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: ERROR: subtransaction logged without previous top-level txn record
Date: Mon, 10 Feb 2020 16:04:46 +0300
Message-ID: <87wo8ulhjl.fsf@ars-thinkpad> (raw)
In-Reply-To: <CAA4eK1Kcsib6UG7zFPrL-h2fvnByfKWxq8Xvzg=2hxUebYwt=g@mail.gmail.com>
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>
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.
-- cheers, Arseny
Attachments:
[text/x-diff] subxact_logged_without_top_test.patch (1.7K, ../87wo8ulhjl.fsf@ars-thinkpad/2-subxact_logged_without_top_test.patch)
download | inline diff:diff --git a/contrib/test_decoding/specs/subxact_logged_without_top.spec b/contrib/test_decoding/specs/subxact_logged_without_top.specnew file mode 100644index 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"
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.sher@postgrespro.ru, amit.kapila16@gmail.com, andres@anarazel.de, hsuchen@amazon.com, pgsql-bugs@lists.postgresql.org
Subject: Re: ERROR: subtransaction logged without previous top-level txn record
In-Reply-To: <87wo8ulhjl.fsf@ars-thinkpad>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox