Re: Augment every test postgresql.conf

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.

> 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.