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(at)dunslane(dot)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(at)sss(dot)pgh(dot)pa(dot)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