Hi, On 2021-12-31 11:25:28 -0800, Andres Freund wrote: > cfbot now runs most tests on windows, the windows task is by far the slowest, > and the task limitted most in concurrency [2]. Running tap tests is the > biggest part of that. This is a bigger issue on windows because we don't have > infrastructure (yet) to run tests in parallel. > > There's a few tests which stand out in their slowness, which seem worth > addressing even if we tackle test parallelism on windows at some point. I > often find them to be the slowest tests on linux too. > > Picking a random successful cfbot run [1] I see the following tap tests taking > more than 20 seconds: > > 67188 ms pg_basebackup t/010_pg_basebackup.pl > 25751 ms pg_verifybackup t/002_algorithm.pl
The reason these in particular are slow is that they do a lot of pg_basebackups without either / one-of -cfast / --no-sync. The lack of -cfast in particularly is responsible for a significant proportion of the test time. The only reason this didn't cause the tests to take many minutes is that spread checkpoints only throttle when writing out a buffer and there aren't that many dirty buffers... Attached is a patch changing the parameters in all the instances I found. Testing on a local instance it about halves the runtime of t/010_pg_basebackup.pl on linux and windows (but there's still a 2x time difference between the two), it's less when running the tests concurrently CI. It might be worth having one explicit use of -cspread. Perhaps combined with an explicit checkpoint beforehand? Greetings, Andres Freund
>From 4abed85e9085786f82d87667f2451821c01d37c0 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Mon, 17 Jan 2022 00:51:43 -0800 Subject: [PATCH v1 1/4] tests: Consistently use pg_basebackup -cfast --no-sync to accelerate tests. Most tests invoking pg_basebackup themselves did not yet use -cfast, which makes pg_basebackup take considerably longer. The only reason this didn't cause the tests to take many minutes is that spread checkpoints only throttle when writing out a buffer and there aren't that many dirty buffers... Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 104 ++++++++++--------- src/bin/pg_verifybackup/t/002_algorithm.pl | 2 +- src/bin/pg_verifybackup/t/003_corruption.pl | 2 +- src/bin/pg_verifybackup/t/004_options.pl | 2 +- src/bin/pg_verifybackup/t/006_encoding.pl | 2 +- src/bin/pg_verifybackup/t/007_wal.pl | 4 +- 6 files changed, 61 insertions(+), 55 deletions(-) diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 872fec3d350..ebd102cd098 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -20,6 +20,13 @@ my $tempdir = PostgreSQL::Test::Utils::tempdir; my $node = PostgreSQL::Test::Cluster->new('main'); +# For nearly all tests some options should be specified, to keep test times +# reasonable. Using @pg_basebackup_defs as the first element of the array +# passed to to IPC::Run interpolates the array (as it is not a reference to an +# array).. +my @pg_basebackup_defs = ('pg_basebackup', '--no-sync', '-cfast'); + + # Set umask so test directories and files are created with default permissions umask(0077); @@ -43,7 +50,7 @@ $node->set_replication_conf(); system_or_bail 'pg_ctl', '-D', $pgdata, 'reload'; $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup" ], 'pg_basebackup fails because of WAL configuration'); ok(!-d "$tempdir/backup", 'backup directory was cleaned up'); @@ -54,7 +61,7 @@ mkdir("$tempdir/backup") or BAIL_OUT("unable to create $tempdir/backup"); append_to_file("$tempdir/backup/dir-not-empty.txt", "Some data"); -$node->command_fails([ 'pg_basebackup', '-D', "$tempdir/backup", '-n' ], +$node->command_fails([ @pg_basebackup_defs, '-D', "$tempdir/backup", '-n' ], 'failing run with no-clean option'); ok(-d "$tempdir/backup", 'backup directory was created and left behind'); @@ -105,7 +112,7 @@ foreach my $filename (@tempRelationFiles) } # Run base backup. -$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backup", '-X', 'none' ], +$node->command_ok([ @pg_basebackup_defs, '-D', "$tempdir/backup", '-X', 'none' ], 'pg_basebackup runs'); ok(-f "$tempdir/backup/PG_VERSION", 'backup was created'); ok(-f "$tempdir/backup/backup_manifest", 'backup manifest included'); @@ -165,9 +172,9 @@ rmtree("$tempdir/backup"); $node->command_ok( [ - 'pg_basebackup', '-D', - "$tempdir/backup2", '--no-manifest', - '--waldir', "$tempdir/xlog2" + @pg_basebackup_defs, '-D', + "$tempdir/backup2", '--no-manifest', + '--waldir', "$tempdir/xlog2" ], 'separate xlog directory'); ok(-f "$tempdir/backup2/PG_VERSION", 'backup was created'); @@ -176,31 +183,31 @@ ok(-d "$tempdir/xlog2/", 'xlog directory was created'); rmtree("$tempdir/backup2"); rmtree("$tempdir/xlog2"); -$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup", '-Ft' ], +$node->command_ok([ @pg_basebackup_defs, '-D', "$tempdir/tarbackup", '-Ft' ], 'tar format'); ok(-f "$tempdir/tarbackup/base.tar", 'backup tar was created'); rmtree("$tempdir/tarbackup"); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T=/foo" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-T=/foo" ], '-T with empty old directory fails'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=" ], '-T with empty new directory fails'); $node->command_fails( [ - 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', + @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=/bar=/baz" ], '-T with multiple = fails'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo=/bar" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo=/bar" ], '-T with old directory not absolute fails'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=bar" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=bar" ], '-T with new directory not absolute fails'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo" ], '-T with invalid format fails'); # Tar format doesn't support filenames longer than 100 bytes. @@ -211,7 +218,7 @@ open my $file, '>', "$superlongpath" or die "unable to create file $superlongpath"; close $file; $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/tarbackup_l1", '-Ft' ], + [ @pg_basebackup_defs, '-D', "$tempdir/tarbackup_l1", '-Ft' ], 'pg_basebackup tar with long name fails'); unlink "$pgdata/$superlongname"; @@ -329,14 +336,14 @@ foreach my $filename (@tempRelationFiles) } $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup1", '-Fp' ], 'plain format with tablespaces fails without tablespace mapping'); $node->command_ok( [ - 'pg_basebackup', '-D', - "$tempdir/backup1", '-Fp', - "-T$realTsDir=$real_tempdir/tbackup/tblspc1" + @pg_basebackup_defs, '-D', + "$tempdir/backup1", '-Fp', + "-T$realTsDir=$real_tempdir/tbackup/tblspc1", ], 'plain format with tablespaces succeeds with tablespace mapping'); ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated'); @@ -404,9 +411,9 @@ $node->safe_psql('postgres', $realTsDir =~ s/=/\\=/; $node->command_ok( [ - 'pg_basebackup', '-D', - "$tempdir/backup3", '-Fp', - "-T$realTsDir=$real_tempdir/tbackup/tbl\\=spc2" + @pg_basebackup_defs, '-D', + "$tempdir/backup3", '-Fp', + "-T$realTsDir=$real_tempdir/tbackup/tbl\\=spc2", ], 'mapping tablespace with = sign in path'); ok(-d "$tempdir/tbackup/tbl=spc2", 'tablespace with = sign was relocated'); @@ -417,12 +424,12 @@ mkdir "$tempdir/$superlongname"; $realTsDir = "$real_sys_tempdir/$superlongname"; $node->safe_psql('postgres', "CREATE TABLESPACE tblspc3 LOCATION '$realTsDir';"); -$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ], +$node->command_ok([ @pg_basebackup_defs, '-D', "$tempdir/tarbackup_l3", '-Ft' ], 'pg_basebackup tar with long symlink target'); $node->safe_psql('postgres', "DROP TABLESPACE tblspc3;"); rmtree("$tempdir/tarbackup_l3"); -$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ], +$node->command_ok([ @pg_basebackup_defs, '-D', "$tempdir/backupR", '-R' ], 'pg_basebackup -R runs'); ok(-f "$tempdir/backupR/postgresql.auto.conf", 'postgresql.auto.conf exists'); ok(-f "$tempdir/backupR/standby.signal", 'standby.signal was created'); @@ -436,32 +443,32 @@ like( 'postgresql.auto.conf sets primary_conninfo'); $node->command_ok( - [ 'pg_basebackup', '-D', "$tempdir/backupxd" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxd" ], 'pg_basebackup runs in default xlog mode'); ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxd/pg_wal")), 'WAL files copied'); rmtree("$tempdir/backupxd"); $node->command_ok( - [ 'pg_basebackup', '-D', "$tempdir/backupxf", '-X', 'fetch' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxf", '-X', 'fetch' ], 'pg_basebackup -X fetch runs'); ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_wal")), 'WAL files copied'); rmtree("$tempdir/backupxf"); $node->command_ok( - [ 'pg_basebackup', '-D', "$tempdir/backupxs", '-X', 'stream' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxs", '-X', 'stream' ], 'pg_basebackup -X stream runs'); ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxs/pg_wal")), 'WAL files copied'); rmtree("$tempdir/backupxs"); $node->command_ok( - [ 'pg_basebackup', '-D', "$tempdir/backupxst", '-X', 'stream', '-Ft' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxst", '-X', 'stream', '-Ft' ], 'pg_basebackup -X stream runs in tar mode'); ok(-f "$tempdir/backupxst/pg_wal.tar", "tar file was created"); rmtree("$tempdir/backupxst"); $node->command_ok( [ - 'pg_basebackup', '-D', + @pg_basebackup_defs, '-D', "$tempdir/backupnoslot", '-X', 'stream', '--no-slot' ], @@ -470,7 +477,7 @@ rmtree("$tempdir/backupnoslot"); $node->command_fails( [ - 'pg_basebackup', '-D', + @pg_basebackup_defs, '-D', "$tempdir/backupxs_sl_fail", '-X', 'stream', '-S', 'slot0' @@ -478,12 +485,12 @@ $node->command_fails( 'pg_basebackup fails with nonexistent replication slot'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxs_slot", '-C' ], 'pg_basebackup -C fails without slot name'); $node->command_fails( [ - 'pg_basebackup', '-D', + @pg_basebackup_defs, '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0', '--no-slot' @@ -491,7 +498,7 @@ $node->command_fails( 'pg_basebackup fails with -C -S --no-slot'); $node->command_ok( - [ 'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0' ], 'pg_basebackup -C runs'); rmtree("$tempdir/backupxs_slot"); @@ -510,7 +517,7 @@ isnt( 'restart LSN of new slot is not null'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/backupxs_slot1", '-C', '-S', 'slot0' ], + [ @pg_basebackup_defs, '-D', "$tempdir/backupxs_slot1", '-C', '-S', 'slot0' ], 'pg_basebackup fails with -C -S and a previously existing slot'); $node->safe_psql('postgres', @@ -520,12 +527,12 @@ my $lsn = $node->safe_psql('postgres', ); is($lsn, '', 'restart LSN of new slot is null'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1', '-X', 'none' ], + [ @pg_basebackup_defs, '-D', "$tempdir/fail", '-S', 'slot1', '-X', 'none' ], 'pg_basebackup with replication slot fails without WAL streaming'); $node->command_ok( [ - 'pg_basebackup', '-D', "$tempdir/backupxs_sl", '-X', - 'stream', '-S', 'slot1' + @pg_basebackup_defs, '-D', "$tempdir/backupxs_sl", '-X', + 'stream', '-S', 'slot1' ], 'pg_basebackup -X stream with replication slot runs'); $lsn = $node->safe_psql('postgres', @@ -536,8 +543,8 @@ rmtree("$tempdir/backupxs_sl"); $node->command_ok( [ - 'pg_basebackup', '-D', "$tempdir/backupxs_sl_R", '-X', - 'stream', '-S', 'slot1', '-R' + @pg_basebackup_defs, '-D', "$tempdir/backupxs_sl_R", '-X', + 'stream', '-S', 'slot1', '-R', ], 'pg_basebackup with replication slot and -R runs'); like( @@ -570,7 +577,7 @@ close $file; system_or_bail 'pg_ctl', '-D', $pgdata, 'start'; $node->command_checks_all( - [ 'pg_basebackup', '-D', "$tempdir/backup_corrupt" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt" ], 1, [qr{^$}], [qr/^WARNING.*checksum verification failed/s], @@ -590,7 +597,7 @@ close $file; system_or_bail 'pg_ctl', '-D', $pgdata, 'start'; $node->command_checks_all( - [ 'pg_basebackup', '-D', "$tempdir/backup_corrupt2" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt2" ], 1, [qr{^$}], [qr/^WARNING.*further.*failures.*will.not.be.reported/s], @@ -606,7 +613,7 @@ close $file; system_or_bail 'pg_ctl', '-D', $pgdata, 'start'; $node->command_checks_all( - [ 'pg_basebackup', '-D', "$tempdir/backup_corrupt3" ], + [ @pg_basebackup_defs, '-D', "$tempdir/backup_corrupt3" ], 1, [qr{^$}], [qr/^WARNING.*7 total checksum verification failures/s], @@ -616,8 +623,8 @@ rmtree("$tempdir/backup_corrupt3"); # do not verify checksums, should return ok $node->command_ok( [ - 'pg_basebackup', '-D', - "$tempdir/backup_corrupt4", '--no-verify-checksums' + @pg_basebackup_defs, '-D', + "$tempdir/backup_corrupt4", '--no-verify-checksums', ], 'pg_basebackup with -k does not report checksum mismatch'); rmtree("$tempdir/backup_corrupt4"); @@ -635,18 +642,17 @@ SKIP: $node->command_ok( [ - 'pg_basebackup', '-D', + @pg_basebackup_defs, '-D', "$tempdir/backup_gzip", '--compress', - '1', '--no-sync', - '--format', 't' + '1', '--format', + 't' ], 'pg_basebackup with --compress'); $node->command_ok( [ - 'pg_basebackup', '-D', + @pg_basebackup_defs, '-D', "$tempdir/backup_gzip2", '--gzip', - '--no-sync', '--format', - 't' + '--format', 't' ], 'pg_basebackup with --gzip'); diff --git a/src/bin/pg_verifybackup/t/002_algorithm.pl b/src/bin/pg_verifybackup/t/002_algorithm.pl index 06333c6c0d5..5d9965ba8f4 100644 --- a/src/bin/pg_verifybackup/t/002_algorithm.pl +++ b/src/bin/pg_verifybackup/t/002_algorithm.pl @@ -21,7 +21,7 @@ for my $algorithm (qw(bogus none crc32c sha224 sha256 sha384 sha512)) my $backup_path = $primary->backup_dir . '/' . $algorithm; my @backup = ( 'pg_basebackup', '-D', $backup_path, - '--manifest-checksums', $algorithm, '--no-sync'); + '--manifest-checksums', $algorithm, '--no-sync', '-cfast'); my @verify = ('pg_verifybackup', '-e', $backup_path); # A backup with a bogus algorithm should fail. diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl index 63cad6647b5..53be2efd87d 100644 --- a/src/bin/pg_verifybackup/t/003_corruption.pl +++ b/src/bin/pg_verifybackup/t/003_corruption.pl @@ -114,7 +114,7 @@ for my $scenario (@scenario) local $ENV{MSYS2_ARG_CONV_EXCL} = $source_ts_prefix; $primary->command_ok( [ - 'pg_basebackup', '-D', $backup_path, '--no-sync', + 'pg_basebackup', '-D', $backup_path, '--no-sync', '-cfast', '-T', "${source_ts_path}=${backup_ts_path}" ], "base backup ok"); diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl index 3ec4651b680..5a8fd9b0ecb 100644 --- a/src/bin/pg_verifybackup/t/004_options.pl +++ b/src/bin/pg_verifybackup/t/004_options.pl @@ -17,7 +17,7 @@ my $primary = PostgreSQL::Test::Cluster->new('primary'); $primary->init(allows_streaming => 1); $primary->start; my $backup_path = $primary->backup_dir . '/test_options'; -$primary->command_ok([ 'pg_basebackup', '-D', $backup_path, '--no-sync' ], +$primary->command_ok([ 'pg_basebackup', '-D', $backup_path, '--no-sync', '-cfast' ], "base backup ok"); # Verify that pg_verifybackup -q succeeds and produces no output. diff --git a/src/bin/pg_verifybackup/t/006_encoding.pl b/src/bin/pg_verifybackup/t/006_encoding.pl index e746ae6dcc4..b869cd8fe6e 100644 --- a/src/bin/pg_verifybackup/t/006_encoding.pl +++ b/src/bin/pg_verifybackup/t/006_encoding.pl @@ -19,7 +19,7 @@ $primary->command_ok( [ 'pg_basebackup', '-D', $backup_path, '--no-sync', - '--manifest-force-encode' + '-cfast', '--manifest-force-encode' ], "backup ok with forced hex encoding"); diff --git a/src/bin/pg_verifybackup/t/007_wal.pl b/src/bin/pg_verifybackup/t/007_wal.pl index b9282da7078..4723047d73d 100644 --- a/src/bin/pg_verifybackup/t/007_wal.pl +++ b/src/bin/pg_verifybackup/t/007_wal.pl @@ -17,7 +17,7 @@ my $primary = PostgreSQL::Test::Cluster->new('primary'); $primary->init(allows_streaming => 1); $primary->start; my $backup_path = $primary->backup_dir . '/test_wal'; -$primary->command_ok([ 'pg_basebackup', '-D', $backup_path, '--no-sync' ], +$primary->command_ok([ 'pg_basebackup', '-D', $backup_path, '--no-sync', '-cfast' ], "base backup ok"); # Rename pg_wal. @@ -71,7 +71,7 @@ $primary->safe_psql('postgres', 'SELECT pg_switch_wal()'); my $backup_path2 = $primary->backup_dir . '/test_tli'; # The base backup run below does a checkpoint, that removes the first segment # of the current timeline. -$primary->command_ok([ 'pg_basebackup', '-D', $backup_path2, '--no-sync' ], +$primary->command_ok([ 'pg_basebackup', '-D', $backup_path2, '--no-sync', '-cfast' ], "base backup 2 ok"); command_ok( [ 'pg_verifybackup', $backup_path2 ], -- 2.34.0
