Hi,
This thread started on committers, at
https://www.postgresql.org/message-id/20200818234532.uiafo5br5lo6zhya%40alap3.anarazel.de
In it I wanted to add a isolation test around prepared transactions:
On 2020-08-18 16:45:32 -0700, Andres Freund wrote:
> I think it's worth adding an isolation test. But it doesn't seem like
> extending prepared-transactions.spec makes too much sense, it doesn't
> fit in well. It's a lot easier to reproduce the issue without
> SERIALIZABLE, for example. Generally the file seems more about
> serializable than 2PC...
>
> So unless somebody disagrees I'm gonna add a new
> prepared-transactions-2.spec.
But I noticed that the already existing prepared transactions test
wasn't in the normal schedule, since:
commit ae55d9fbe3871a5e6309d9b91629f1b0ff2b8cba
Author: Andrew Dunstan <and...@dunslane.net>
Date: 2012-07-20 15:51:40 -0400
Remove prepared transactions from main isolation test schedule.
There is no point in running this test when prepared transactions are
disabled,
which is the default. New make targets that include the test are provided.
This
will save some useless waste of cycles on buildfarm machines.
Backpatch to 9.1 where these tests were introduced.
diff --git a/src/test/isolation/isolation_schedule
b/src/test/isolation/isolation_schedule
index 669c0f220c4..2184975dcb1 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -9,7 +9,6 @@ test: ri-trigger
test: partial-index
test: two-ids
test: multiple-row-versions
-test: prepared-transactions
test: fk-contention
test: fk-deadlock
test: fk-deadlock2
The commit confuses me, cause I thought we explicitly enabled prepared
transactions during tests well before that? See
commit 8d4f2ecd41312e57422901952cbad234d293060b
Author: Tom Lane <t...@sss.pgh.pa.us>
Date: 2009-04-23 00:23:46 +0000
Change the default value of max_prepared_transactions to zero, and add
documentation warnings against setting it nonzero unless active use of
prepared transactions is intended and a suitable transaction manager has
been
installed. This should help to prevent the type of scenario we've seen
several times now where a prepared transaction is forgotten and eventually
causes severe maintenance problems (or even anti-wraparound shutdown).
The only real reason we had the default be nonzero in the first place was to
support regression testing of the feature. To still be able to do that,
tweak pg_regress to force a nonzero value during "make check". Since we
cannot force a nonzero value in "make installcheck", add a variant
regression
test "expected" file that shows the results that will be obtained when
max_prepared_transactions is zero.
Also, extend the HINT messages for transaction wraparound warnings to
mention
the possibility that old prepared transactions are causing the problem.
All per today's discussion.
And indeed, including the test in the schedule works for make check, not
just an installcheck with explicitly enabled prepared xacts.
ISTM we should just add an alternative output for disabled prepared
xacts, and re-add the test?
My new test, without the alternative output for now, is attached.
Greetings,
Andres Freund
diff --git c/src/test/isolation/expected/prepared-transactions-2.out i/src/test/isolation/expected/prepared-transactions-2.out
new file mode 100644
index 00000000000..f7fd2e7f989
--- /dev/null
+++ i/src/test/isolation/expected/prepared-transactions-2.out
@@ -0,0 +1,119 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_ins_before s1_begin s1_txid s2_txid s1_update_in s1_ins_in s1_show s1_prepare s1_show s1_commit s1_show
+step s1_ins_before: INSERT INTO preptest VALUES ('before');
+step s1_begin: BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
+step s1_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s2_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s1_update_in: UPDATE preptest SET data = 'in; '|| data;
+step s1_ins_in: INSERT INTO preptest VALUES ('in');
+step s1_show: SELECT * FROM preptest
+data
+
+in; before
+in
+step s1_prepare: PREPARE TRANSACTION 'prep';
+step s1_show: SELECT * FROM preptest
+data
+
+before
+step s1_commit: COMMIT PREPARED 'prep';
+step s1_show: SELECT * FROM preptest
+data
+
+in; before
+in
+
+starting permutation: s1_ins_before s1_begin s1_txid s2_txid s1_update_in s1_ins_in s1_show s1_prepare s2_show s1_commit s2_show
+step s1_ins_before: INSERT INTO preptest VALUES ('before');
+step s1_begin: BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
+step s1_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s2_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s1_update_in: UPDATE preptest SET data = 'in; '|| data;
+step s1_ins_in: INSERT INTO preptest VALUES ('in');
+step s1_show: SELECT * FROM preptest
+data
+
+in; before
+in
+step s1_prepare: PREPARE TRANSACTION 'prep';
+step s2_show: SELECT * FROM preptest
+data
+
+before
+step s1_commit: COMMIT PREPARED 'prep';
+step s2_show: SELECT * FROM preptest
+data
+
+in; before
+in
+
+starting permutation: s1_ins_before s1_begin s1_txid s2_txid s1_update_in s1_ins_in s1_show s1_prepare s1_show s1_rollback s1_show
+step s1_ins_before: INSERT INTO preptest VALUES ('before');
+step s1_begin: BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
+step s1_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s2_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s1_update_in: UPDATE preptest SET data = 'in; '|| data;
+step s1_ins_in: INSERT INTO preptest VALUES ('in');
+step s1_show: SELECT * FROM preptest
+data
+
+in; before
+in
+step s1_prepare: PREPARE TRANSACTION 'prep';
+step s1_show: SELECT * FROM preptest
+data
+
+before
+step s1_rollback: ROLLBACK PREPARED 'prep';
+step s1_show: SELECT * FROM preptest
+data
+
+before
+
+starting permutation: s1_ins_before s1_begin s1_txid s2_txid s1_update_in s1_ins_in s1_show s1_prepare s2_show s1_rollback s2_show
+step s1_ins_before: INSERT INTO preptest VALUES ('before');
+step s1_begin: BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
+step s1_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s2_txid: SELECT txid_current() IS NULL;
+?column?
+
+f
+step s1_update_in: UPDATE preptest SET data = 'in; '|| data;
+step s1_ins_in: INSERT INTO preptest VALUES ('in');
+step s1_show: SELECT * FROM preptest
+data
+
+in; before
+in
+step s1_prepare: PREPARE TRANSACTION 'prep';
+step s2_show: SELECT * FROM preptest
+data
+
+before
+step s1_rollback: ROLLBACK PREPARED 'prep';
+step s2_show: SELECT * FROM preptest
+data
+
+before
diff --git c/src/test/isolation/isolation_schedule i/src/test/isolation/isolation_schedule
index 218c87b24bf..ec15ce784db 100644
--- c/src/test/isolation/isolation_schedule
+++ i/src/test/isolation/isolation_schedule
@@ -1,3 +1,5 @@
+test: prepared-transactions
+test: prepared-transactions-2
test: read-only-anomaly
test: read-only-anomaly-2
test: read-only-anomaly-3
diff --git c/src/test/isolation/specs/prepared-transactions-2.spec i/src/test/isolation/specs/prepared-transactions-2.spec
new file mode 100644
index 00000000000..d5ed58e4cfc
--- /dev/null
+++ i/src/test/isolation/specs/prepared-transactions-2.spec
@@ -0,0 +1,60 @@
+# This test checks against a bug found in the initial support for
+# caching snapshots. The bug caused the snapshot from within the
+# to-be-prepared transaction to be used after preparing, erroneously
+# not including the prepared transaction's xid, thereby effectively
+# treating it as aborted.
+#
+# See also https://postgr.es/m/E1k7tGP-0005V0-5k%40gemulon.postgresql.org
+
+setup
+{
+ CREATE TABLE preptest(data text);
+}
+
+teardown
+{
+ DROP TABLE preptest;
+}
+
+session "s1"
+step "s1_begin" { BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED; }
+step "s1_txid" { SELECT txid_current() IS NULL; }
+step "s1_ins_before" { INSERT INTO preptest VALUES ('before'); }
+step "s1_update_in" { UPDATE preptest SET data = 'in; '|| data; }
+step "s1_ins_in" { INSERT INTO preptest VALUES ('in'); }
+step "s1_prepare" { PREPARE TRANSACTION 'prep'; }
+step "s1_commit" { COMMIT PREPARED 'prep'; }
+step "s1_rollback" { ROLLBACK PREPARED 'prep'; }
+step "s1_show" { SELECT * FROM preptest }
+
+session "s2"
+step "s2_show" { SELECT * FROM preptest }
+step "s2_txid" { SELECT txid_current() IS NULL; }
+
+# This permutation shows the bug
+permutation "s1_ins_before" "s1_begin" "s1_txid"
+ # Without acquiring an xid here, s1's xid would be the snapshot's
+ # xmax, hiding the bug (as only xids < xmax are looked up in ->xip).
+ "s2_txid"
+ "s1_update_in" "s1_ins_in" "s1_show" "s1_prepare"
+ # With the bug the next select would find xmax neither committed nor in progress,
+ # and therefore mark xmax as invalid. Obviously breaking visibility.
+ "s1_show" "s1_commit" "s1_show"
+
+# The following are just variations of the theme that seem good to
+# also test, even though they weren't affected by the bug.
+
+permutation "s1_ins_before" "s1_begin" "s1_txid"
+ "s2_txid"
+ "s1_update_in" "s1_ins_in" "s1_show" "s1_prepare"
+ "s2_show" "s1_commit" "s2_show"
+
+permutation "s1_ins_before" "s1_begin" "s1_txid"
+ "s2_txid"
+ "s1_update_in" "s1_ins_in" "s1_show" "s1_prepare"
+ "s1_show" "s1_rollback" "s1_show"
+
+permutation "s1_ins_before" "s1_begin" "s1_txid"
+ "s2_txid"
+ "s1_update_in" "s1_ins_in" "s1_show" "s1_prepare"
+ "s2_show" "s1_rollback" "s2_show"
