On 8/18/20 9:22 PM, Andres Freund wrote: > 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?
here's the context for the 2012 commit. https://www.postgresql.org/message-id/flat/50099220.2060005%40dunslane.net#8b189efc4920e1996ffa4d6a0ef81b9c So I hope any changes that are made will not result in a major slowdown of buildfarm animals. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
