v4-0001-Rework-the-option-set-of-pg_basebackup.patch

From 1bbe624a4bea5e00a1997f81fe3b1ec1f637fa44 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Sat, 15 Jan 2022 11:51:40 +0900
Subject: [PATCH v4] Rework the option set of pg_basebackup

---
 src/bin/pg_basebackup/pg_basebackup.c        | 99 ++++++++++++++------
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 37 ++++----
 doc/src/sgml/ref/pg_basebackup.sgml          | 33 ++++---
 3 files changed, 105 insertions(+), 64 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index aa43fc0924..57b054a8ad 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -113,6 +113,7 @@ static bool showprogress = false;
 static bool estimatesize = true;
 static int	verbose = 0;
 static int	compresslevel = 0;
+static WalCompressionMethod client_method = COMPRESSION_NONE;
 static IncludeWal includewal = STREAM_WAL;
 static bool fastcheckpoint = false;
 static bool writerecoveryconf = false;
@@ -358,8 +359,10 @@ usage(void)
 	printf(_("      --waldir=WALDIR    location for the write-ahead log directory\n"));
 	printf(_("  -X, --wal-method=none|fetch|stream\n"
 			 "                         include required WAL files with specified method\n"));
-	printf(_("  -z, --gzip             compress tar output\n"));
-	printf(_("  -Z, --compress=0-9     compress tar output with given compression level\n"));
+	printf(_("      --client-compress=METHOD\n"
+			 "                         method to compress data (client-side)\n"));
+	printf(_("      --compression-level=1-9\n"
+			 "                         compress tar output with given compression level\n"));
 	printf(_("\nGeneral options:\n"));
 	printf(_("  -c, --checkpoint=fast|spread\n"
 			 "                         set fast or spread checkpointing\n"));
@@ -524,8 +527,7 @@ LogStreamerMain(logstreamer_param *param)
 													stream.do_sync);
 	else
 		stream.walmethod = CreateWalTarMethod(param->xlog,
-											  (compresslevel != 0) ?
-											  COMPRESSION_GZIP : COMPRESSION_NONE,
+											  client_method,
 											  compresslevel,
 											  stream.do_sync);
 
@@ -976,7 +978,7 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 					 bool is_recovery_guc_supported,
 					 bool expect_unterminated_tarfile)
 {
-	bbstreamer *streamer;
+	bbstreamer *streamer = NULL;
 	bbstreamer *manifest_inject_streamer = NULL;
 	bool		inject_manifest;
 	bool		must_parse_archive;
@@ -1035,19 +1037,22 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 			archive_file = NULL;
 		}
 
+		if (client_method == COMPRESSION_NONE)
+			streamer = bbstreamer_plain_writer_new(archive_filename,
+												   archive_file);
 #ifdef HAVE_LIBZ
-		if (compresslevel != 0)
+		else if (client_method == COMPRESSION_GZIP)
 		{
 			strlcat(archive_filename, ".gz", sizeof(archive_filename));
 			streamer = bbstreamer_gzip_writer_new(archive_filename,
 												  archive_file,
 												  compresslevel);
 		}
-		else
 #endif
-			streamer = bbstreamer_plain_writer_new(archive_filename,
-												   archive_file);
-
+		else
+		{
+			Assert(false);		/* not reachable */
+		}
 
 		/*
 		 * If we need to parse the archive for whatever reason, then we'll
@@ -1745,8 +1750,6 @@ main(int argc, char **argv)
 		{"slot", required_argument, NULL, 'S'},
 		{"tablespace-mapping", required_argument, NULL, 'T'},
 		{"wal-method", required_argument, NULL, 'X'},
-		{"gzip", no_argument, NULL, 'z'},
-		{"compress", required_argument, NULL, 'Z'},
 		{"label", required_argument, NULL, 'l'},
 		{"no-clean", no_argument, NULL, 'n'},
 		{"no-sync", no_argument, NULL, 'N'},
@@ -1766,6 +1769,8 @@ main(int argc, char **argv)
 		{"no-manifest", no_argument, NULL, 5},
 		{"manifest-force-encode", no_argument, NULL, 6},
 		{"manifest-checksums", required_argument, NULL, 7},
+		{"client-compress", required_argument, NULL, 8},
+		{"compression-level", required_argument, NULL, 9},
 		{NULL, 0, NULL, 0}
 	};
 	int			c;
@@ -1793,7 +1798,7 @@ main(int argc, char **argv)
 
 	atexit(cleanup_directories_atexit);
 
-	while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
+	while ((c = getopt_long(argc, argv, "CD:F:r:RS:T:X:l:nNd:c:h:p:U:s:wWkvP",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -1872,18 +1877,6 @@ main(int argc, char **argv)
 			case 'N':
 				do_sync = false;
 				break;
-			case 'z':
-#ifdef HAVE_LIBZ
-				compresslevel = Z_DEFAULT_COMPRESSION;
-#else
-				compresslevel = 1;	/* will be rejected below */
-#endif
-				break;
-			case 'Z':
-				if (!option_parse_int(optarg, "-Z/--compress", 0, 9,
-									  &compresslevel))
-					exit(1);
-				break;
 			case 'c':
 				if (pg_strcasecmp(optarg, "fast") == 0)
 					fastcheckpoint = true;
@@ -1942,6 +1935,24 @@ main(int argc, char **argv)
 			case 7:
 				manifest_checksums = pg_strdup(optarg);
 				break;
+			case 8:
+				if (pg_strcasecmp(optarg, "gzip") == 0)
+					client_method = COMPRESSION_GZIP;
+				else if (pg_strcasecmp(optarg, "none") == 0)
+					client_method = COMPRESSION_NONE;
+				else
+				{
+					pg_log_error("invalid value \"%s\" for option %s",
+								 optarg, "--client-compress");
+					exit(1);
+				}
+				break;
+			case 9:
+				if (!option_parse_int(optarg, "--compression-level", 1, 9,
+									  &compresslevel))
+					exit(1);
+				break;
+				break;
 			default:
 
 				/*
@@ -1979,7 +1990,7 @@ main(int argc, char **argv)
 	/*
 	 * Mutually exclusive arguments
 	 */
-	if (format == 'p' && compresslevel != 0)
+	if (format == 'p' && client_method != COMPRESSION_NONE)
 	{
 		pg_log_error("only tar mode backups can be compressed");
 		fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
@@ -2057,13 +2068,39 @@ main(int argc, char **argv)
 		}
 	}
 
-#ifndef HAVE_LIBZ
-	if (compresslevel != 0)
+	/*
+	 * Compression-related options.
+	 */
+	switch (client_method)
 	{
-		pg_log_error("this build does not support compression");
-		exit(1);
-	}
+		case COMPRESSION_NONE:
+			if (compresslevel != 0)
+			{
+				pg_log_error("cannot use --compression-level with --client-compress=%s",
+							 "none");
+				fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+						progname);
+				exit(1);
+			}
+			break;
+		case COMPRESSION_GZIP:
+#ifdef HAVE_LIBZ
+			if (compresslevel == 0)
+			{
+				pg_log_info("no value specified for --compression-level, switching to default");
+				compresslevel = Z_DEFAULT_COMPRESSION;
+			}
+#else
+			pg_log_error("this build does not support compression with %s",
+						 "gzip");
+			exit(1);
 #endif
+			break;
+		case COMPRESSION_LZ4:
+			/* option not supported */
+			Assert(false);
+			break;
+	}
 
 	if (showprogress && !estimatesize)
 	{
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 872fec3d35..0f84b3952a 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -31,6 +31,17 @@ my $pgdata = $node->data_dir;
 $node->command_fails(['pg_basebackup'],
 	'pg_basebackup needs target directory specified');
 
+# Sanity checks for options
+$node->command_fails_like(
+	[
+		'pg_basebackup',   '-D',
+		"$tempdir/backup", '--client-compress',
+		'none',            '--compression-level',
+		'1'
+	],
+	qr/\Qpg_basebackup: error: cannot use --compression-level with --client-compress=none/,
+	'failure if --compression-level specified with --client-compress=none');
+
 # Some Windows ANSI code pages may reject this filename, in which case we
 # quietly proceed without this bit of test coverage.
 if (open my $badchars, '>>', "$tempdir/pgdata/FOO\xe0\xe0\xe0BAR")
@@ -630,23 +641,14 @@ note "Testing pg_basebackup with compression methods";
 # Check ZLIB compression if available.
 SKIP:
 {
-	skip "postgres was not built with ZLIB support", 5
+	skip "postgres was not built with ZLIB support", 3
 	  if (!check_pg_config("#define HAVE_LIBZ 1"));
 
 	$node->command_ok(
 		[
 			'pg_basebackup',        '-D',
-			"$tempdir/backup_gzip", '--compress',
-			'1',                    '--no-sync',
-			'--format',             't'
-		],
-		'pg_basebackup with --compress');
-	$node->command_ok(
-		[
-			'pg_basebackup',         '-D',
-			"$tempdir/backup_gzip2", '--gzip',
-			'--no-sync',             '--format',
-			't'
+			"$tempdir/backup_gzip", '--client-compress',
+			'gzip',                 '--no-sync', '--format', 't'
 		],
 		'pg_basebackup with --gzip');
 
@@ -654,11 +656,8 @@ SKIP:
 	# names.
 	my @zlib_files = glob "$tempdir/backup_gzip/*.tar.gz";
 	is(scalar(@zlib_files), 2,
-		"two files created with --compress (base.tar.gz and pg_wal.tar.gz)");
-	my @zlib_files2 = glob "$tempdir/backup_gzip2/*.tar.gz";
-	is(scalar(@zlib_files2), 2,
-		"two files created with --gzip (base.tar.gz and pg_wal.tar.gz)");
-
+		"two files created with --client-compress=gzip (base.tar.gz and pg_wal.tar.gz)"
+	);
 	# Check the integrity of the files generated.
 	my $gzip = $ENV{GZIP_PROGRAM};
 	skip "program gzip is not found in your system", 1
@@ -666,9 +665,7 @@ SKIP:
 		|| $gzip eq ''
 		|| system_log($gzip, '--version') != 0);
 
-	my $gzip_is_valid =
-	  system_log($gzip, '--test', @zlib_files, @zlib_files2);
+	my $gzip_is_valid = system_log($gzip, '--test', @zlib_files);
 	is($gzip_is_valid, 0, "gzip verified the integrity of compressed data");
 	rmtree("$tempdir/backup_gzip");
-	rmtree("$tempdir/backup_gzip2");
 }
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 9e6807b457..1a859794b2 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -354,28 +354,35 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
-      <term><option>-z</option></term>
-      <term><option>--gzip</option></term>
+      <term><option>--compression-level=<replaceable class="parameter">level</replaceable></option></term>
       <listitem>
        <para>
-        Enables gzip compression of tar file output, with the default
-        compression level. Compression is only available when using
-        the tar format, and the suffix <filename>.gz</filename> will
-        automatically be added to all tar filenames.
+        Controls the level of compression used with the tar file output,
+        and specifies the compression level (1 through 9, 1 being worst
+        compression and 9 being best compression). Compression is only
+        available when using the tar format, and the suffix
+        <filename>.gz</filename> will automatically be added to all tar
+        filenames.
+       </para>
+       <para>
+        This option is only available with <literal>--client-compress</literal>
+        set to <literal>gzip</literal>.
        </para>
       </listitem>
      </varlistentry>
 
      <varlistentry>
-      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
-      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
+      <term><option>--client-compress=<replaceable class="parameter">method</replaceable></option></term>
       <listitem>
        <para>
-        Enables gzip compression of tar file output, and specifies the
-        compression level (0 through 9, 0 being no compression and 9 being best
-        compression). Compression is only available when using the tar
-        format, and the suffix <filename>.gz</filename> will
-        automatically be added to all tar filenames.
+        Enables client-side compression of backup data using the specified
+        method. Supported values are <literal>gzip</literal> and
+        <literal>none</literal>, the default.
+       </para>
+
+       <para>
+        The suffix <filename>.gz</filename> will automatically be added to
+        all filenames when using <literal>gzip</literal>.
        </para>
       </listitem>
      </varlistentry>
-- 
2.34.1