From 3820c63da8d0e59e2bd4476e91968f03be5dd041 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <andrew@dunslane.net>
Date: Mon, 10 Apr 2017 19:53:47 -0400
Subject: [PATCH] Run most pg_dump and pg_dumpall tests with --no-sync

Commit 96a7128b made pg_dump and pg_dumpall sync their output by
default. However, there's no great need for that in testing, and it
could impose a performance penalty, so we add the --no-sync flag to most
of the test cases.

Michael Paquier
---
 src/bin/pg_dump/t/002_pg_dump.pl            | 96 +++++++++++++++------
 src/bin/pg_dump/t/010_dump_connstr.pl       | 14 +--
 src/bin/pg_upgrade/test.sh                  |  4 +-
 src/test/modules/test_pg_dump/t/001_base.pl | 79 ++++++++++++-----
 4 files changed, 136 insertions(+), 57 deletions(-)

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 5030bc204c7..cccad04ab62 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -39,6 +39,7 @@ my %pgdump_runs = (
 	binary_upgrade => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			'--format=custom',
 			"--file=$tempdir/binary_upgrade.dump",
 			'-w',
@@ -55,6 +56,7 @@ my %pgdump_runs = (
 	clean => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/clean.sql",
 			'--include-subscriptions',
 			'-c',
@@ -63,6 +65,7 @@ my %pgdump_runs = (
 	clean_if_exists => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/clean_if_exists.sql",
 			'--include-subscriptions',
 			'-c',
@@ -71,12 +74,16 @@ my %pgdump_runs = (
 			'postgres', ], },
 	column_inserts => {
 		dump_cmd => [
-			'pg_dump', "--file=$tempdir/column_inserts.sql",
-			'-a',      '--column-inserts',
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/column_inserts.sql",
+			'-a',
+		    '--column-inserts',
 			'postgres', ], },
 	createdb => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/createdb.sql",
 			'--include-subscriptions',
 			'-C',
@@ -86,6 +93,7 @@ my %pgdump_runs = (
 	data_only => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/data_only.sql",
 			'--include-subscriptions',
 			'-a',
@@ -94,8 +102,13 @@ my %pgdump_runs = (
 			'-v',                 # no-op, just make sure it works
 			'postgres', ], },
 	defaults => {
-		dump_cmd => [ 'pg_dump', '-f', "$tempdir/defaults.sql", 'postgres', ],
-	},
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			'-f',
+			"$tempdir/defaults.sql",
+			'postgres', ], },
+	# Do not use --no-sync to give test coverage for data sync.
 	defaults_custom_format => {
 		test_key => 'defaults',
 		dump_cmd => [
@@ -105,6 +118,7 @@ my %pgdump_runs = (
 			'pg_restore', '-Fc',
 			"--file=$tempdir/defaults_custom_format.sql",
 			"$tempdir/defaults_custom_format.dump", ], },
+	# Do not use --no-sync to give test coverage for data sync.
 	defaults_dir_format => {
 		test_key => 'defaults',
 		dump_cmd => [
@@ -114,6 +128,7 @@ my %pgdump_runs = (
 			'pg_restore', '-Fd',
 			"--file=$tempdir/defaults_dir_format.sql",
 			"$tempdir/defaults_dir_format", ], },
+	# Do not use --no-sync to give test coverage for data sync.
 	defaults_parallel => {
 		test_key => 'defaults',
 		dump_cmd => [
@@ -123,6 +138,7 @@ my %pgdump_runs = (
 			'pg_restore',
 			"--file=$tempdir/defaults_parallel.sql",
 			"$tempdir/defaults_parallel", ], },
+	# Do not use --no-sync to give test coverage for data sync.
 	defaults_tar_format => {
 		test_key => 'defaults',
 		dump_cmd => [
@@ -135,17 +151,22 @@ my %pgdump_runs = (
 			"$tempdir/defaults_tar_format.tar", ], },
 	exclude_dump_test_schema => {
 		dump_cmd => [
-			'pg_dump', "--file=$tempdir/exclude_dump_test_schema.sql",
-			'--exclude-schema=dump_test', 'postgres', ], },
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/exclude_dump_test_schema.sql",
+			'--exclude-schema=dump_test',
+			'postgres', ], },
 	exclude_test_table => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/exclude_test_table.sql",
 			'--exclude-table=dump_test.test_table',
 			'postgres', ], },
 	exclude_test_table_data => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/exclude_test_table_data.sql",
 			'--exclude-table-data=dump_test.test_table',
 			'--no-unlogged-table-data',
@@ -153,30 +174,52 @@ my %pgdump_runs = (
 	pg_dumpall_globals => {
 		dump_cmd => [
 			'pg_dumpall',                             '-v',
-			"--file=$tempdir/pg_dumpall_globals.sql", '-g', ], },
+			"--file=$tempdir/pg_dumpall_globals.sql", '-g',
+			'--no-sync', ], },
 	pg_dumpall_globals_clean => {
 		dump_cmd => [
-			'pg_dumpall', "--file=$tempdir/pg_dumpall_globals_clean.sql",
-			'-g',         '-c', ], },
+			'pg_dumpall',
+			"--file=$tempdir/pg_dumpall_globals_clean.sql",
+			'-g',
+			'-c',
+			'--no-sync', ], },
 	pg_dumpall_dbprivs => {
-		dump_cmd =>
-		  [ 'pg_dumpall', "--file=$tempdir/pg_dumpall_dbprivs.sql", ], },
+		dump_cmd => [
+			'pg_dumpall',
+			'--no-sync',
+			"--file=$tempdir/pg_dumpall_dbprivs.sql", ], },
 	no_blobs => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/no_blobs.sql", '-B', 'postgres', ], },
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/no_blobs.sql",
+			'-B',
+			'postgres', ], },
 	no_privs => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/no_privs.sql", '-x', 'postgres', ], },
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/no_privs.sql",
+			'-x',
+			'postgres', ], },
 	no_owner => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/no_owner.sql", '-O', 'postgres', ], },
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/no_owner.sql",
+			'-O',
+			'postgres', ], },
 	only_dump_test_schema => {
 		dump_cmd => [
-			'pg_dump', "--file=$tempdir/only_dump_test_schema.sql",
-			'--schema=dump_test', 'postgres', ], },
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/only_dump_test_schema.sql",
+			'--schema=dump_test',
+			'postgres', ], },
 	only_dump_test_table => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/only_dump_test_table.sql",
 			'--table=dump_test.test_table',
 			'--lock-wait-timeout=1000000',
@@ -184,6 +227,7 @@ my %pgdump_runs = (
 	role => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/role.sql",
 			'--role=regress_dump_test_role',
 			'--schema=dump_test_second_schema',
@@ -192,6 +236,7 @@ my %pgdump_runs = (
 		test_key => 'role',
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			'--format=directory',
 			'--jobs=2',
 			"--file=$tempdir/role_parallel",
@@ -204,28 +249,29 @@ my %pgdump_runs = (
 	schema_only => {
 		dump_cmd => [
 			'pg_dump', '--format=plain', "--file=$tempdir/schema_only.sql",
-			'-s', 'postgres', ], },
+			'--no-sync', '-s', 'postgres', ], },
 	section_pre_data => {
 		dump_cmd => [
 			'pg_dump',            "--file=$tempdir/section_pre_data.sql",
 			'--include-subscriptions',
-			'--section=pre-data', 'postgres', ], },
+			'--section=pre-data', '--no-sync', 'postgres', ], },
 	section_data => {
 		dump_cmd => [
 			'pg_dump',        "--file=$tempdir/section_data.sql",
-			'--section=data', 'postgres', ], },
+			'--section=data', '--no-sync', 'postgres', ], },
 	section_post_data => {
 		dump_cmd => [
 			'pg_dump',             "--file=$tempdir/section_post_data.sql",
-			'--section=post-data', 'postgres', ], },
+			'--section=post-data', '--no-sync', 'postgres', ], },
 	test_schema_plus_blobs => {
 		dump_cmd => [
 			'pg_dump', "--file=$tempdir/test_schema_plus_blobs.sql",
-			'--schema=dump_test', '-b', '-B', 'postgres', ], },
+
+			'--schema=dump_test', '-b', '-B', '--no-sync', 'postgres', ], },
 	with_oids => {
 		dump_cmd => [
 			'pg_dump',                       '--oids',
-			'--include-subscriptions',
+			'--include-subscriptions',       '--no-sync',
 			"--file=$tempdir/with_oids.sql", 'postgres', ], },);
 
 ###############################################################
diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl
index 81b57792485..55944d10fad 100644
--- a/src/bin/pg_dump/t/010_dump_connstr.pl
+++ b/src/bin/pg_dump/t/010_dump_connstr.pl
@@ -59,29 +59,29 @@ $node->command_ok(
 		'-U', $dbname4 ],
 	'pg_dumpall with long ASCII name 1');
 $node->command_ok(
-	[   'pg_dumpall', '-r', '-f', $discard, '--dbname',
+	[   'pg_dumpall', '--no-sync', '-r', '-f', $discard, '--dbname',
 		$node->connstr($dbname2),
 		'-U', $dbname3 ],
 	'pg_dumpall with long ASCII name 2');
 $node->command_ok(
-	[   'pg_dumpall', '-r', '-f', $discard, '--dbname',
+	[   'pg_dumpall', '--no-sync', '-r', '-f', $discard, '--dbname',
 		$node->connstr($dbname3),
 		'-U', $dbname2 ],
 	'pg_dumpall with long ASCII name 3');
 $node->command_ok(
-	[   'pg_dumpall', '-r', '-f', $discard, '--dbname',
+	[   'pg_dumpall', '--no-sync', '-r', '-f', $discard, '--dbname',
 		$node->connstr($dbname4),
 		'-U', $dbname1 ],
 	'pg_dumpall with long ASCII name 4');
 $node->command_ok(
-	[ 'pg_dumpall', '-r', '-l', 'dbname=template1' ],
+	[ 'pg_dumpall', '--no-sync', '-r', '-l', 'dbname=template1' ],
 	'pg_dumpall -l accepts connection string');
 
 $node->run_log([ 'createdb', "foo\n\rbar" ]);
 
 # not sufficient to use -r here
 $node->command_fails(
-	[ 'pg_dumpall', '-f', $discard ],
+	[ 'pg_dumpall', '--no-sync', '-f', $discard ],
 	'pg_dumpall with \n\r in database name');
 $node->run_log([ 'dropdb', "foo\n\rbar" ]);
 
@@ -91,7 +91,7 @@ $node->safe_psql($dbname1, 'CREATE TABLE t0()');
 
 # XXX no printed message when this fails, just SIGPIPE termination
 $node->command_ok(
-	[   'pg_dump', '-Fd', '-j2', '-f', $dirfmt,
+	[   'pg_dump', '-Fd', '--no-sync', '-j2', '-f', $dirfmt,
 		'-U', $dbname1, $node->connstr($dbname1) ],
 	'parallel dump');
 
@@ -112,7 +112,7 @@ $node->command_ok(
 	'parallel restore with create');
 
 
-$node->command_ok([ 'pg_dumpall', '-f', $plain, '-U', $dbname1 ],
+$node->command_ok([ 'pg_dumpall', '--no-sync', '-f', $plain, '-U', $dbname1 ],
 	'take full dump');
 system_log('cat', $plain);
 my ($stderr, $result);
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index cbc52595502..841da034b09 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -170,7 +170,7 @@ createdb "$dbname2" || createdb_status=$?
 createdb "$dbname3" || createdb_status=$?
 
 if "$MAKE" -C "$oldsrc" installcheck; then
-	pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
+	pg_dumpall --no-sync -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
 	if [ "$newsrc" != "$oldsrc" ]; then
 		oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
 		fix_sql=""
@@ -221,7 +221,7 @@ case $testhost in
 	*)		sh ./analyze_new_cluster.sh ;;
 esac
 
-pg_dumpall -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
+pg_dumpall --no-sync -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
 pg_ctl -m fast stop
 
 # no need to echo commands anymore
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index 7b3955aac99..3e45ccb005f 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -41,16 +41,21 @@ my $tempdir_short = TestLib::tempdir_short;
 my %pgdump_runs = (
 	binary_upgrade => {
 		dump_cmd => [
-			'pg_dump',       "--file=$tempdir/binary_upgrade.sql",
-			'--schema-only', '--binary-upgrade',
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/binary_upgrade.sql",
+			'--schema-only',
+			'--binary-upgrade',
 			'--dbname=postgres', ], },
 	clean => {
 		dump_cmd => [
 			'pg_dump', "--file=$tempdir/clean.sql",
-			'-c',      '--dbname=postgres', ], },
+			'-c',      '--no-sync',
+			'--dbname=postgres', ], },
 	clean_if_exists => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/clean_if_exists.sql",
 			'-c',
 			'--if-exists',
@@ -58,12 +63,16 @@ my %pgdump_runs = (
 			'postgres', ], },
 	column_inserts => {
 		dump_cmd => [
-			'pg_dump', "--file=$tempdir/column_inserts.sql",
-			'-a',      '--column-inserts',
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/column_inserts.sql",
+			'-a',
+			'--column-inserts',
 			'postgres', ], },
 	createdb => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/createdb.sql",
 			'-C',
 			'-R',                 # no-op, just for testing
@@ -71,6 +80,7 @@ my %pgdump_runs = (
 	data_only => {
 		dump_cmd => [
 			'pg_dump',
+			'--no-sync',
 			"--file=$tempdir/data_only.sql",
 			'-a',
 			'-v',                 # no-op, just make sure it works
@@ -81,7 +91,7 @@ my %pgdump_runs = (
 	defaults_custom_format => {
 		test_key => 'defaults',
 		dump_cmd => [
-			'pg_dump', '-Fc', '-Z6',
+			'pg_dump', '--no-sync', '-Fc', '-Z6',
 			"--file=$tempdir/defaults_custom_format.dump", 'postgres', ],
 		restore_cmd => [
 			'pg_restore',
@@ -90,7 +100,7 @@ my %pgdump_runs = (
 	defaults_dir_format => {
 		test_key => 'defaults',
 		dump_cmd => [
-			'pg_dump',                             '-Fd',
+			'pg_dump', '--no-sync', '-Fd',
 			"--file=$tempdir/defaults_dir_format", 'postgres', ],
 		restore_cmd => [
 			'pg_restore',
@@ -99,8 +109,8 @@ my %pgdump_runs = (
 	defaults_parallel => {
 		test_key => 'defaults',
 		dump_cmd => [
-			'pg_dump', '-Fd', '-j2', "--file=$tempdir/defaults_parallel",
-			'postgres', ],
+			'pg_dump', '--no-sync', '-Fd', '-j2',
+			"--file=$tempdir/defaults_parallel", 'postgres', ],
 		restore_cmd => [
 			'pg_restore',
 			"--file=$tempdir/defaults_parallel.sql",
@@ -108,37 +118,60 @@ my %pgdump_runs = (
 	defaults_tar_format => {
 		test_key => 'defaults',
 		dump_cmd => [
-			'pg_dump',                                 '-Ft',
+			'pg_dump', '--no-sync', '-Ft',
 			"--file=$tempdir/defaults_tar_format.tar", 'postgres', ],
 		restore_cmd => [
 			'pg_restore',
 			"--file=$tempdir/defaults_tar_format.sql",
 			"$tempdir/defaults_tar_format.tar", ], },
 	pg_dumpall_globals => {
-		dump_cmd =>
-		  [ 'pg_dumpall', "--file=$tempdir/pg_dumpall_globals.sql", '-g', ],
+		dump_cmd => [
+			'pg_dumpall',
+			'--no-sync',
+			"--file=$tempdir/pg_dumpall_globals.sql",
+			'-g', ],
 	},
 	no_privs => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/no_privs.sql", '-x', 'postgres', ], },
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/no_privs.sql",
+			'-x',
+			'postgres', ], },
 	no_owner => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/no_owner.sql", '-O', 'postgres', ], },
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/no_owner.sql",
+			'-O',
+			'postgres', ], },
 	schema_only => {
-		dump_cmd =>
-		  [ 'pg_dump', "--file=$tempdir/schema_only.sql", '-s', 'postgres', ],
+		dump_cmd => [
+			'pg_dump',
+			'--no-sync',
+			"--file=$tempdir/schema_only.sql",
+			'-s',
+			'postgres', ],
 	},
 	section_pre_data => {
 		dump_cmd => [
-			'pg_dump',            "--file=$tempdir/section_pre_data.sql",
-			'--section=pre-data', 'postgres', ], },
+			'pg_dump',
+			'--no-sync',
+            "--file=$tempdir/section_pre_data.sql",
+			'--section=pre-data',
+			'postgres', ], },
 	section_data => {
 		dump_cmd => [
-			'pg_dump',        "--file=$tempdir/section_data.sql",
-			'--section=data', 'postgres', ], },
+			'pg_dump',
+			'--no-sync',
+	        "--file=$tempdir/section_data.sql",
+			'--section=data',
+			'postgres', ], },
 	section_post_data => {
 		dump_cmd => [
-			'pg_dump',             "--file=$tempdir/section_post_data.sql",
+			'pg_dump',
+			'--no-sync',
+             "--file=$tempdir/section_post_data.sql",
 			'--section=post-data', 'postgres', ], },);
 
 ###############################################################
-- 
GitLab