Re: Augment every test postgresql.conf

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(at)leadboat(dot)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