On Sat, Dec 29, 2018 at 10:46:31PM -0500, Tom Lane wrote:
> Noah Misch <n...@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.

> Also, if we're to do this, it seems like applying it to back branches
> would be helpful --- but will it work in all the back branches?

Yes.  TEMP_CONFIG exists in all supported branches, and the back-patch to 9.4
is no more complex.  Before 9.6 (commit 87cc6b5), TEMP_CONFIG affected "make
check" and the pg_upgrade test suite, but it did not affect other pg_regress
suites like contrib/* and src/pl/*.  We could back-patch commit 87cc6b5, if
there's demand.  I don't personally need it, because the tests I want to
influence are all TAP tests.

Reply via email to