Rework compression options of pg_receivewal

Since babbbb5 and the introduction of LZ4 in pg_receivewal, the
compression of the WAL archived is controlled by two options:
- --compression-method with "gzip", "none" or "lz4" as possible value.
- --compress=N to specify a compression level.  This includes a
backward-incompatible change where a value of 0 leads to a failure
instead of no compression enforced.

This commit takes advantage of a4b5754 and 3603f7c to rework the
compression options of pg_receivewal, as of:
- The removal of --compression-method.
- The extenction of --compress to use the same grammar as pg_basebackup,
with a METHOD:DETAIL format, where a METHOD is "gzip", "none" or "lz4"
and a DETAIL is a comma-separated list of options, the only keyword
supported is now "level" to control the compression level.  If only an
integer is specified as value of this option, "none" is implied on 0
and "gzip" is implied otherwise.  This brings back --compress to be
backward-compatible with ~14, while still supporting LZ4.

This has also the advantage of centralizing the set of checks used by
pg_receivewal to validate its compression options.

Author: Michael Paquier
Reviewed-by: Robert Haas, Georgios Kokolatos
Discussion: https://postgr.es/m/YlPQGNAAa04raObK@paquier.xyz

doc/src/sgml/ref/pg_receivewal.sgml

@@ -263,15 +263,32 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
-      <term><option>--compression-method=<replaceable class="parameter">method</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
+      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
+      <term><option>--compress=<replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
       <listitem>
        <para>
-        Enables compression of write-ahead logs using the specified method.
+        Enables compression of write-ahead logs.
-        Supported values are <literal>gzip</literal>, <literal>lz4</literal>
+       </para>
-        (if <productname>PostgreSQL</productname> was compiled with
+       <para>
-        <option>--with-lz4</option>), and <literal>none</literal>.
+        The compression method can be set to <literal>gzip</literal>,
+        <literal>lz4</literal> (if <productname>PostgreSQL</productname>
+        was compiled with <option>--with-lz4</option>) or
+        <literal>none</literal> for no compression.
+        A compression detail string can optionally be specified.  If the
+        detail string is an integer, it specifies the compression level.
+        Otherwise, it should be a comma-separated list of items, each of the
+        form <literal>keyword</literal> or <literal>keyword=value</literal>.
+        Currently, the only supported keyword is <literal>level</literal>.
+       </para>
+       <para>
+        If no compression level is specified, the default compression level
+        will be used. If only a level is specified without mentioning an
+        algorithm, <literal>gzip</literal> compression will be used if the
+        level is greater than 0, and no compression will be used if the level
+        is 0.
        </para>
-
        <para>
         The suffix <filename>.gz</filename> will automatically be added to
         all filenames when using <literal>gzip</literal>, and the suffix
@@ -279,24 +296,6 @@ PostgreSQL documentation
        </para>
       </listitem>
      </varlistentry>
-
-     <varlistentry>
-      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
-      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
-      <listitem>
-       <para>
-        Specifies the compression level (<literal>1</literal> through
-        <literal>9</literal>, <literal>1</literal> being worst compression
-        and <literal>9</literal> being best compression) for WAL segments
-        compressed with <application>gzip</application>.
-       </para>
-
-       <para>
-        This option requires <option>--compression-method</option> to be
-        specified with <literal>gzip</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
     </variablelist>
 
    <para>

src/bin/pg_basebackup/pg_receivewal.c

@@ -57,6 +57,8 @@ static XLogRecPtr endpos = InvalidXLogRecPtr;
 
 
 static void usage(void);
+static void parse_compress_options(char *option, char **algorithm,
+								   char **detail);
 static DIR *get_destination_dir(char *dest_folder);
 static void close_destination_dir(DIR *dest_dir, char *dest_folder);
 static XLogRecPtr FindStreamingStart(uint32 *tli);
@@ -90,9 +92,8 @@ usage(void)
 	printf(_("      --synchronous      flush write-ahead log immediately after writing\n"));
 	printf(_("  -v, --verbose          output verbose messages\n"));
 	printf(_("  -V, --version          output version information, then exit\n"));
-	printf(_("      --compression-method=METHOD\n"
+	printf(_("  -Z, --compress=METHOD[:DETAIL]\n"
-			 "                         method to compress logs\n"));
+			 "                         compress as specified\n"));
-	printf(_("  -Z, --compress=1-9     compress logs with given compression level\n"));
 	printf(_("  -?, --help             show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -d, --dbname=CONNSTR   connection string\n"));
@@ -108,6 +109,66 @@ usage(void)
 	printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL);
 }
 
+/*
+ * Basic parsing of a value specified for -Z/--compress
+ *
+ * The parsing consists of a METHOD:DETAIL string fed later on to a more
+ * advanced routine in charge of proper validation checks.  This only extracts
+ * METHOD and DETAIL.  If only an integer is found, the method is implied by
+ * the value specified.
+ */
+static void
+parse_compress_options(char *option, char **algorithm, char **detail)
+{
+	char	   *sep;
+	char	   *endp;
+	long		result;
+
+	/*
+	 * Check whether the compression specification consists of a bare integer.
+	 *
+	 * For backward-compatibility, assume "none" if the integer found is zero
+	 * and "gzip" otherwise.
+	 */
+	result = strtol(option, &endp, 10);
+	if (*endp == '\0')
+	{
+		if (result == 0)
+		{
+			*algorithm = pstrdup("none");
+			*detail = NULL;
+		}
+		else
+		{
+			*algorithm = pstrdup("gzip");
+			*detail = pstrdup(option);
+		}
+		return;
+	}
+
+	/*
+	 * Check whether there is a compression detail following the algorithm
+	 * name.
+	 */
+	sep = strchr(option, ':');
+	if (sep == NULL)
+	{
+		*algorithm = pstrdup(option);
+		*detail = NULL;
+	}
+	else
+	{
+		char	   *alg;
+
+		alg = palloc((sep - option) + 1);
+		memcpy(alg, option, sep - option);
+		alg[sep - option] = '\0';
+
+		*algorithm = alg;
+		*detail = pstrdup(sep + 1);
+	}
+}
+
 /*
  * Check if the filename looks like a WAL file, letting caller know if this
  * WAL segment is partial and/or compressed.
@@ -651,7 +712,6 @@ main(int argc, char **argv)
 		{"if-not-exists", no_argument, NULL, 3},
 		{"synchronous", no_argument, NULL, 4},
 		{"no-sync", no_argument, NULL, 5},
-		{"compression-method", required_argument, NULL, 6},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -660,6 +720,10 @@ main(int argc, char **argv)
 	char	   *db_name;
 	uint32		hi,
 				lo;
+	pg_compress_specification compression_spec;
+	char	   *compression_detail = NULL;
+	char	   *compression_algorithm_str = "none";
+	char	   *error_detail = NULL;
 
 	pg_logging_init(argv[0]);
 	progname = get_progname(argv[0]);
@@ -728,9 +792,8 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				if (!option_parse_int(optarg, "-Z/--compress", 1, 9,
+				parse_compress_options(optarg, &compression_algorithm_str,
-									  &compresslevel))
+									   &compression_detail);
-					exit(1);
 				break;
 /* action */
 			case 1:
@@ -748,17 +811,6 @@ main(int argc, char **argv)
 			case 5:
 				do_sync = false;
 				break;
-			case 6:
-				if (pg_strcasecmp(optarg, "gzip") == 0)
-					compression_algorithm = PG_COMPRESSION_GZIP;
-				else if (pg_strcasecmp(optarg, "lz4") == 0)
-					compression_algorithm = PG_COMPRESSION_LZ4;
-				else if (pg_strcasecmp(optarg, "none") == 0)
-					compression_algorithm = PG_COMPRESSION_NONE;
-				else
-					pg_fatal("invalid value \"%s\" for option %s",
-							 optarg, "--compression-method");
-				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -810,24 +862,33 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-
 	/*
-	 * Compression-related options.
+	 * Compression options
 	 */
+	if (!parse_compress_algorithm(compression_algorithm_str,
+								  &compression_algorithm))
+		pg_fatal("unrecognized compression algorithm \"%s\"",
+				 compression_algorithm_str);
+
+	parse_compress_specification(compression_algorithm, compression_detail,
+								 &compression_spec);
+	error_detail = validate_compress_specification(&compression_spec);
+	if (error_detail != NULL)
+		pg_fatal("invalid compression specification: %s",
+				 error_detail);
+
+	/* Extract the compression level, if found in the specification */
+	if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) != 0)
+		compresslevel = compression_spec.level;
+
 	switch (compression_algorithm)
 	{
 		case PG_COMPRESSION_NONE:
-			if (compresslevel != 0)
+			/* nothing to do */
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "none");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
 			break;
 		case PG_COMPRESSION_GZIP:
 #ifdef HAVE_LIBZ
-			if (compresslevel == 0)
+			if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) == 0)
 			{
 				pg_log_info("no value specified for --compress, switching to default");
 				compresslevel = Z_DEFAULT_COMPRESSION;
@@ -838,15 +899,7 @@ main(int argc, char **argv)
 #endif
 			break;
 		case PG_COMPRESSION_LZ4:
-#ifdef USE_LZ4
+#ifndef USE_LZ4
-			if (compresslevel != 0)
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "lz4");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
-#else
 			pg_fatal("this build does not support compression with %s",
 					 "LZ4");
 #endif

src/bin/pg_basebackup/t/020_pg_receivewal.pl

@@ -34,12 +34,9 @@ $primary->command_fails(
 	[ 'pg_receivewal', '-D', $stream_dir, '--synchronous', '--no-sync' ],
 	'failure if --synchronous specified with --no-sync');
 $primary->command_fails_like(
-	[
+	[ 'pg_receivewal', '-D', $stream_dir, '--compress', 'none:1', ],
-		'pg_receivewal', '-D', $stream_dir, '--compression-method', 'none',
+	qr/\Qpg_receivewal: error: invalid compression specification: compression algorithm "none" does not accept a compression level/,
-		'--compress',    '1'
+	'failure if --compress none:N (where N > 0)');
-	],
-	qr/\Qpg_receivewal: error: cannot use --compress with --compression-method=none/,
-	'failure if --compress specified with --compression-method=none');
 
 # Slot creation and drop
 my $slot_name = 'test';
@@ -48,7 +45,7 @@ $primary->command_ok(
 	'creating a replication slot');
 my $slot = $primary->slot($slot_name);
 is($slot->{'slot_type'}, 'physical', 'physical replication slot was created');
-is($slot->{'restart_lsn'}, '', 'restart LSN of new slot is null');
+is($slot->{'restart_lsn'}, '',       'restart LSN of new slot is null');
 $primary->command_ok([ 'pg_receivewal', '--slot', $slot_name, '--drop-slot' ],
 	'dropping a replication slot');
 is($primary->slot($slot_name)->{'slot_type'},
@@ -93,15 +90,10 @@ SKIP:
 	chomp($nextlsn);
 	$primary->psql('postgres', 'INSERT INTO test_table VALUES (2);');
 
-	# Note the trailing whitespace after the value of --compress, that is
-	# a valid value.
 	$primary->command_ok(
 		[
-			'pg_receivewal',        '-D',
+			'pg_receivewal', '-D',     $stream_dir,  '--verbose',
-			$stream_dir,            '--verbose',
+			'--endpos',      $nextlsn, '--compress', 'gzip:1',
-			'--endpos',             $nextlsn,
-			'--compression-method', 'gzip',
-			'--compress',           '1 ',
 			'--no-loop'
 		],
 		"streaming some WAL using ZLIB compression");
@@ -153,13 +145,11 @@ SKIP:
 	# Stream up to the given position.
 	$primary->command_ok(
 		[
-			'pg_receivewal', '-D',
+			'pg_receivewal', '-D',     $stream_dir, '--verbose',
-			$stream_dir,     '--verbose',
+			'--endpos',      $nextlsn, '--no-loop', '--compress',
-			'--endpos',      $nextlsn,
-			'--no-loop',     '--compression-method',
 			'lz4'
 		],
-		'streaming some WAL using --compression-method=lz4');
+		'streaming some WAL using --compress=lz4');
 
 	# Verify that the stored files are generated with their expected
 	# names.