From: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Subject: Re: Augment every test postgresql.conf
To: Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
References: <20181229021950.GA3302966@rfd.leadboat.com>
 <20181230024014.GA181668@gust.leadboat.com> <16623.1546141591@sss.pgh.pa.us>
 <20181230055346.GB156585@gust.leadboat.com>
Openpgp: preference=signencrypt
Autocrypt: addr=andrew.dunstan@2ndQuadrant.com; prefer-encrypt=mutual;
 keydata=
 xsBNBE7KWFkBCAClridxur2AIc7eW2AR7izbfp3EnNefie2HbLF0izW5Ik5UjX2HBXBx4syI
 gY6b0ugohXrr274+baoAlvSbq6cAoQuEVrk5IZFzt20b1Xkx65FwGSEj526yiKLocqkJceSq
 Xr9xcA5SGY+FZv441chh5SU92v4q6z+6LPpoHOh97ptAVXZYNTtU0LevyvD5lja0TzbvJm6C
 eFXitJfnm1pLEr0DGJCR/iUOl/N62Kh4855zZC7NHIjQHPOvV5Stz/l5ilDhvGVk+xkXFPys
 SjZoUr1rXhYLpiyi5sR0X9FHXT0KnGuz1F5ERO7ZTLSSQ6fJwPj6gOk9K+vvoKvoeql5ABEB
 AAHNL0FuZHJldyBEdW5zdGFuIDxhbmRyZXcuZHVuc3RhbkAybmRRdWFkcmFudC5jb20+wsCY
 BBMBAgBCAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAUJDyV6hBYhBOQ+WEYd/Hy/RGkV
 pZn6f8tZ/DuBBQJcH/EVAhkBAAoJEJn6f8tZ/DuB7BEH/iDWZEbVft2m2MT6SPjARzTupt/l
 xLLZ/Cl6n2US2hov5+fBbnR5xvECk/YCOfG9ICvKNlpFjceqNLRKStd8cmndnI+kDw91uNM2
 hpL4aJMeW65XCHvqXlOLSwv5chBpXz9NG7FMaJ+q+59sWEWjcTNmjXgkEHn53y+1pc7nSLDe
 cr5ZI1aB3nGXRm1SoDY5PYZkUKMykW+YBZwgcs/Q5tRaE+siN7DKbz8E628+PerwAkmHPVXN
 MdJFD6gs4S0gDFA8IxcyIwZU2v4By+brzcJ9Se3FK5BYxBDHtaYZxgSrJAvOUOrLIZPQTQfL
 TtnVs3C3JTfrxCH1ceHtnA6dRuTOwE0ETspYWQEIANGc4zQULOxhbqO2dyD51YhqCNRmm9oK
 Waqf+wmW4tpDe/VVcxAnNizd4LWCHfzpb5cHAtGkOPePMfzWVf6nvdF7d3eglbtf59+zG7O7
 llV0xSSoFiieQBsrGvqDInXYX/4mRRXMtyhM353/tixC9RWLs1oofyYmCPPXXY7h9R7en3B8
 BoVrRFcdzlIY/NFNhFGW/9dkEiGjgna2Rk6e15kln4ZvFBWUg23p93w/pqXcxY6+k/8TEk+C
 4R+M6w7o2PLGOjdZ+kPiUcw5H85zf/yZJwQXzisXaNduwWB6Vads9YC9dj6kPR1c4VGRqAaY
 L++LAEOqrlvm2TvqQqZRtnEAEQEAAcLAZQQYAQIADwIbDAUCXA6fpAUJDyV6xAAKCRCZ+n/L
 Wfw7geU4B/9vFFsVmWxerDWgNjyiK80zIPmlw/iwdUlU6FIu0JWLMg4093kvxBs7RMZV9hBq
 n3ekBvD4IvFsVLJrw9qzSI2FH3UeYy5WXpEs01Ppia60NOlvRDqXbryzuvfe+iYo6bVP3Ikr
 TPTNYL/BePUTD2DPvSI82SbGTyzuA7SJv6NRqqQDMIy1fU6KyQFLmQJp/QJyhKWzm9jWh8ee
 w8jVaoMY0Kbq5N7RqisEx1Lu/Iw646UphdYzQNymf+kbiU5Kf4I6DhzV8w9sie1Bo1juNj3I
 c5/fFJsmxljbnaDbXPcnFsur1Oo9mIHFakgWgOBMuxoWfjONiLjjLh/9+ZIqSSLX
Message-ID: <f8938ff6-4dc8-2beb-636b-1d524db78e2f@2ndQuadrant.com>
Date: Sun, 30 Dec 2018 10:32:31 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.0
MIME-Version: 1.0
In-Reply-To: <20181230055346.GB156585@gust.leadboat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Content-Language: en-MW
Precedence: bulk


On 12/30/18 12:53 AM, Noah Misch wrote:
> On Sat, Dec 29, 2018 at 10:46:31PM -0500, Tom Lane wrote:
>> Noah Misch <noah@leadboat.com> writes:
>>> Looking more closely, we already have the TEMP_CONFIG variable and apply it to
>>> everything except TAP suites.  Closing that gap, as attached, is enough.  The
>>> buildfarm client uses TEMP_CONFIG to implement its extra_config setting, so
>>> this will cause extra_config to start applying to TAP suites.
>> Seems reasonable, but it might be a good idea to warn the buildfarm-owners
>> list before committing.  (Although I guess it wouldn't be hard to check
>> the buildfarm database to see if anyone is putting anything interesting
>> into their critters' TEMP_CONFIG.)
> Good idea.  Here are the extra_config entries seen since 2018-12-01:
>
>  archive_mode = off
>  force_parallel_mode = regress
>  fsync = off
>  fsync = on
>  jit=1
>  jit = 1
>  jit_above_cost=0
>  jit = on
>  jit_optimize_above_cost=1000
>  log_checkpoints = 'true'
>  log_connections = 'true'
>  log_disconnections = 'true'
>  log_line_prefix = '[%c:%l] '
>  log_line_prefix = '%m [%c:%l] '
>  log_line_prefix = '%m [%c:%l] %q%a '
>  log_line_prefix = '%m [%p:%l] '
>  log_line_prefix = '%m [%p:%l] %q%a '
>  log_line_prefix = '%m [%s %p:%l] %q%a '
>  log_statement = 'all'
>  max_parallel_workers_per_gather = 2
>  max_parallel_workers_per_gather = 5
>  max_wal_senders = 0
>  shared_buffers = 10MB
>  stats_temp_directory = '/cygdrive/w/lorikeet/HEAD'
>  stats_temp_directory = '/cygdrive/w/lorikeet/REL_10_STABLE'
>  stats_temp_directory = '/cygdrive/w/lorikeet/REL_11_STABLE'
>  stats_temp_directory = '/cygdrive/w/lorikeet/REL9_4_STABLE'
>  stats_temp_directory = '/cygdrive/w/lorikeet/REL9_5_STABLE'
>  stats_temp_directory = '/cygdrive/w/lorikeet/REL9_6_STABLE'
>  stats_temp_directory= '/home/buildfarm/data/stats_temp'
>  wal_level = 'minimal'
>
> Problems:
>
> 1. max_wal_senders=0 and wal_level=minimal break a number of suites,
>    e.g. pg_basebackup.
> 2. stats_temp_directory is incompatible with TAP suites that start more than
>    one node simultaneously.
>
> Problem (1) goes away if I inject the TEMP_CONFIG settings earlier in the
> file, which seems defensible:
>
> --- a/src/test/perl/PostgresNode.pm
> +++ b/src/test/perl/PostgresNode.pm
> @@ -447,10 +447,13 @@ sub init
>  	print $conf "log_statement = all\n";
>  	print $conf "log_replication_commands = on\n";
>  	print $conf "wal_retrieve_retry_interval = '500ms'\n";
>  	print $conf "port = $port\n";
>  
> +	print $conf TestLib::slurp_file($ENV{TEMP_CONFIG})
> +	  if defined $ENV{TEMP_CONFIG};
> +
>  	if ($params{allows_streaming})
>  	{
>  		if ($params{allows_streaming} eq "logical")
>  		{
>  			print $conf "wal_level = logical\n";
>
> Problem (2) remains.  It's already a problem for "make -j check-world".  I'll
> give that one more thought.
>


lorikeet is putting the stats_temp directory on a ramdisk. This is worth
testing in any case, but in lorikeet's case was done to help speed up
the tests. When I had a Raspberry Pi instance I did something similar,
for the same reason.


The obvious quick fix would be to have PostgresNode.pm set this to the
default after inserting the TEMP_CONFIG file.


There are a couple of problems here that bear further consideration.
First, that the stats_temp_directory has to exist, and second that there
is no convenient way to make it unique. It would be nice if a) the
directory could be created if it didn't exist and b) some place-holder
in the name could be replaced by a unique identifier such as the node
id. If there is interest I'll work on these. One problem I foresee is
that it might lead to a plethora of stats temp directories being left
around. Still thinking about how we should deal with that. In the
buildfarm client I'd be tempted to create a directory to hold all the
run's stats_temp_directories and then clean it up at the end of the run.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services