From: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
To: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Augment every test postgresql.conf
Date: Sun, 30 Dec 2018 10:32:31 -0500
Message-ID: <f8938ff6-4dc8-2beb-636b-1d524db78e2f@2ndQuadrant.com> (raw)
In-Reply-To: <20181230055346.GB156585@gust.leadboat.com>
References: <20181229021950.GA3302966@rfd.leadboat.com>
<20181230024014.GA181668@gust.leadboat.com>
<16623.1546141591@sss.pgh.pa.us>
<20181230055346.GB156585@gust.leadboat.com>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: andrew.dunstan@2ndquadrant.com, noah@leadboat.com, tgl@sss.pgh.pa.us
Subject: Re: Augment every test postgresql.conf
In-Reply-To: <f8938ff6-4dc8-2beb-636b-1d524db78e2f@2ndQuadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox