Re: prepared transaction isolation tests

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 <andrew@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 <tgl@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