Subject: Re: refactoring basebackup.c

From: Dipesh Pandit <[email-masked]>
Date: Thu, 20 Jan 2022 18:30:43 +0530

Hi,

Thanks for the feedback, I have incorporated the suggestions and
updated a new patch v2.

> I spent some time thinking about test coverage for the server-side
> backup code today and came up with the attached (v12-0003). It does an
> end-to-end test that exercises server-side backup and server-side
> compression and then untars the backup and validity-checks it using
> pg_verifybackup. In addition to being good test coverage for these
> patches, it also plugs a gap in the test coverage of pg_verifybackup,
> which currently has no test case that untars a tar-format backup and
> then verifies the result. I couldn't figure out a way to do that back
> at the time I was working on pg_verifybackup, because I didn't think
> we had any existing precedent for using 'tar' from a TAP test. But it
> was pointed out to me that we do, so I used that as the model for this
> test. It should be easy to generalize this test case to test lz4 and
> zstd as well, I think. But I guess we'll still need something
> different to test what your patch is doing.

I tried to add the test coverage for server side gzip compression with
plain format backup using pg_verifybackup. I have modified the test
to use a flag specific to plain format. If this flag is set then it takes a
plain format backup (with server compression enabled) and verifies
this using pg_verifybackup. I have updated (v2-0002) for the test
coverage.

> It's going to need some documentation changes, too.
yes, I am working on it.

Note: Before applying the patches, please apply Robert's v12 version
of the patches 0001, 0002 and 0003.

Thanks,
Dipesh
From 826a1cbb639afb7e10a20955d3ec64b1bab1fa80 Mon Sep 17 00:00:00 2001
From: Dipesh Pandit <[email-masked]>
Date: Thu, 20 Jan 2022 16:38:36 +0530
Subject: [PATCH 1/2] Support for extracting gzip compressed archive

pg_basebackup can support server side compression using gzip. In
order to support plain format backup with option '-Fp' we need to
add support for decompressing the compressed blocks at client. This
patch addresses the extraction of gzip compressed blocks at client.
---
 src/bin/pg_basebackup/Makefile          |   1 +
 src/bin/pg_basebackup/bbstreamer.h      |   1 +
 src/bin/pg_basebackup/bbstreamer_file.c | 182 ---------------
 src/bin/pg_basebackup/bbstreamer_gzip.c | 377 ++++++++++++++++++++++++++++++++
 src/bin/pg_basebackup/pg_basebackup.c   |  43 +++-
 5 files changed, 419 insertions(+), 185 deletions(-)
 create mode 100644 src/bin/pg_basebackup/bbstreamer_gzip.c

diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index 5b18851..78d96c6 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -38,6 +38,7 @@ OBJS = \
 BBOBJS = \
 	pg_basebackup.o \
 	bbstreamer_file.o \
+	bbstreamer_gzip.o \
 	bbstreamer_inject.o \
 	bbstreamer_tar.o
 
diff --git a/src/bin/pg_basebackup/bbstreamer.h b/src/bin/pg_basebackup/bbstreamer.h
index fc88b50..270b0df 100644
--- a/src/bin/pg_basebackup/bbstreamer.h
+++ b/src/bin/pg_basebackup/bbstreamer.h
@@ -205,6 +205,7 @@ extern bbstreamer *bbstreamer_extractor_new(const char *basepath,
 											const char *(*link_map) (const char *),
 											void (*report_output_file) (const char *));
 
+extern bbstreamer *bbstreamer_gzip_extractor_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_parser_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_terminator_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_archiver_new(bbstreamer *next);
diff --git a/src/bin/pg_basebackup/bbstreamer_file.c b/src/bin/pg_basebackup/bbstreamer_file.c
index 77ca222..d721f87 100644
--- a/src/bin/pg_basebackup/bbstreamer_file.c
+++ b/src/bin/pg_basebackup/bbstreamer_file.c
@@ -11,10 +11,6 @@
 
 #include "postgres_fe.h"
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 #include <unistd.h>
 
 #include "bbstreamer.h"
@@ -30,15 +26,6 @@ typedef struct bbstreamer_plain_writer
 	bool		should_close_file;
 } bbstreamer_plain_writer;
 
-#ifdef HAVE_LIBZ
-typedef struct bbstreamer_gzip_writer
-{
-	bbstreamer	base;
-	char	   *pathname;
-	gzFile		gzfile;
-}			bbstreamer_gzip_writer;
-#endif
-
 typedef struct bbstreamer_extractor
 {
 	bbstreamer	base;
@@ -62,22 +49,6 @@ const bbstreamer_ops bbstreamer_plain_writer_ops = {
 	.free = bbstreamer_plain_writer_free
 };
 
-#ifdef HAVE_LIBZ
-static void bbstreamer_gzip_writer_content(bbstreamer *streamer,
-										   bbstreamer_member *member,
-										   const char *data, int len,
-										   bbstreamer_archive_context context);
-static void bbstreamer_gzip_writer_finalize(bbstreamer *streamer);
-static void bbstreamer_gzip_writer_free(bbstreamer *streamer);
-static const char *get_gz_error(gzFile gzf);
-
-const bbstreamer_ops bbstreamer_gzip_writer_ops = {
-	.content = bbstreamer_gzip_writer_content,
-	.finalize = bbstreamer_gzip_writer_finalize,
-	.free = bbstreamer_gzip_writer_free
-};
-#endif
-
 static void bbstreamer_extractor_content(bbstreamer *streamer,
 										 bbstreamer_member *member,
 										 const char *data, int len,
@@ -196,159 +167,6 @@ bbstreamer_plain_writer_free(bbstreamer *streamer)
 }
 
 /*
- * Create a bbstreamer that just compresses data using gzip, and then writes
- * it to a file.
- *
- * As in the case of bbstreamer_plain_writer_new, pathname is always used
- * for error reporting purposes; if file is NULL, it is also the opened and
- * closed so that the data may be written there.
- */
-bbstreamer *
-bbstreamer_gzip_writer_new(char *pathname, FILE *file, int compresslevel)
-{
-#ifdef HAVE_LIBZ
-	bbstreamer_gzip_writer *streamer;
-
-	streamer = palloc0(sizeof(bbstreamer_gzip_writer));
-	*((const bbstreamer_ops **) &streamer->base.bbs_ops) =
-		&bbstreamer_gzip_writer_ops;
-
-	streamer->pathname = pstrdup(pathname);
-
-	if (file == NULL)
-	{
-		streamer->gzfile = gzopen(pathname, "wb");
-		if (streamer->gzfile == NULL)
-		{
-			pg_log_error("could not create compressed file \"%s\": %m",
-						 pathname);
-			exit(1);
-		}
-	}
-	else
-	{
-		int			fd = dup(fileno(file));
-
-		if (fd < 0)
-		{
-			pg_log_error("could not duplicate stdout: %m");
-			exit(1);
-		}
-
-		streamer->gzfile = gzdopen(fd, "wb");
-		if (streamer->gzfile == NULL)
-		{
-			pg_log_error("could not open output file: %m");
-			exit(1);
-		}
-	}
-
-	if (gzsetparams(streamer->gzfile, compresslevel,
-					Z_DEFAULT_STRATEGY) != Z_OK)
-	{
-		pg_log_error("could not set compression level %d: %s",
-					 compresslevel, get_gz_error(streamer->gzfile));
-		exit(1);
-	}
-
-	return &streamer->base;
-#else
-	pg_log_error("this build does not support compression");
-	exit(1);
-#endif
-}
-
-#ifdef HAVE_LIBZ
-/*
- * Write archive content to gzip file.
- */
-static void
-bbstreamer_gzip_writer_content(bbstreamer *streamer,
-							   bbstreamer_member *member, const char *data,
-							   int len, bbstreamer_archive_context context)
-{
-	bbstreamer_gzip_writer *mystreamer;
-
-	mystreamer = (bbstreamer_gzip_writer *) streamer;
-
-	if (len == 0)
-		return;
-
-	errno = 0;
-	if (gzwrite(mystreamer->gzfile, data, len) != len)
-	{
-		/* if write didn't set errno, assume problem is no disk space */
-		if (errno == 0)
-			errno = ENOSPC;
-		pg_log_error("could not write to compressed file \"%s\": %s",
-					 mystreamer->pathname, get_gz_error(mystreamer->gzfile));
-		exit(1);
-	}
-}
-
-/*
- * End-of-archive processing when writing to a gzip file consists of just
- * calling gzclose.
- *
- * It makes no difference whether we opened the file or the caller did it,
- * because libz provides no way of avoiding a close on the underling file
- * handle. Notice, however, that bbstreamer_gzip_writer_new() uses dup() to
- * work around this issue, so that the behavior from the caller's viewpoint
- * is the same as for bbstreamer_plain_writer.
- */
-static void
-bbstreamer_gzip_writer_finalize(bbstreamer *streamer)
-{
-	bbstreamer_gzip_writer *mystreamer;
-
-	mystreamer = (bbstreamer_gzip_writer *) streamer;
-
-	errno = 0;					/* in case gzclose() doesn't set it */
-	if (gzclose(mystreamer->gzfile) != 0)
-	{
-		pg_log_error("could not close compressed file \"%s\": %m",
-					 mystreamer->pathname);
-		exit(1);
-	}
-
-	mystreamer->gzfile = NULL;
-}
-
-/*
- * Free memory associated with this bbstreamer.
- */
-static void
-bbstreamer_gzip_writer_free(bbstreamer *streamer)
-{
-	bbstreamer_gzip_writer *mystreamer;
-
-	mystreamer = (bbstreamer_gzip_writer *) streamer;
-
-	Assert(mystreamer->base.bbs_next == NULL);
-	Assert(mystreamer->gzfile == NULL);
-
-	pfree(mystreamer->pathname);
-	pfree(mystreamer);
-}
-
-/*
- * Helper function for libz error reporting.
- */
-static const char *
-get_gz_error(gzFile gzf)
-{
-	int			errnum;
-	const char *errmsg;
-
-	errmsg = gzerror(gzf, &errnum);
-	if (errnum == Z_ERRNO)
-		return strerror(errno);
-	else
-		return errmsg;
-}
-#endif
-
-/*
  * Create a bbstreamer that extracts an archive.
  *
  * All pathnames in the archive are interpreted relative to basepath.
diff --git a/src/bin/pg_basebackup/bbstreamer_gzip.c b/src/bin/pg_basebackup/bbstreamer_gzip.c
new file mode 100644
index 0000000..c144a73
--- /dev/null
+++ b/src/bin/pg_basebackup/bbstreamer_gzip.c
@@ -0,0 +1,377 @@
+/*-------------------------------------------------------------------------
+ *
+ * bbstreamer_gzip.c
+ *
+ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		  src/bin/pg_basebackup/bbstreamer_gzip.c
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres_fe.h"
+
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
+#include "bbstreamer.h"
+#include "common/logging.h"
+#include "common/file_perm.h"
+#include "common/string.h"
+
+#ifdef HAVE_LIBZ
+typedef struct bbstreamer_gzip_writer
+{
+	bbstreamer	base;
+	char	   *pathname;
+	gzFile		gzfile;
+}			bbstreamer_gzip_writer;
+
+typedef struct bbstreamer_gzip_extractor
+{
+	bbstreamer	base;
+	z_stream	zstream;
+	size_t		bytes_written;
+} bbstreamer_gzip_extractor;
+
+static void bbstreamer_gzip_writer_content(bbstreamer *streamer,
+										   bbstreamer_member *member,
+										   const char *data, int len,
+										   bbstreamer_archive_context context);
+static void bbstreamer_gzip_writer_finalize(bbstreamer *streamer);
+static void bbstreamer_gzip_writer_free(bbstreamer *streamer);
+static const char *get_gz_error(gzFile gzf);
+
+const bbstreamer_ops bbstreamer_gzip_writer_ops = {
+	.content = bbstreamer_gzip_writer_content,
+	.finalize = bbstreamer_gzip_writer_finalize,
+	.free = bbstreamer_gzip_writer_free
+};
+
+static void bbstreamer_gzip_extractor_content(bbstreamer *streamer,
+											  bbstreamer_member *member,
+											  const char *data, int len,
+											  bbstreamer_archive_context context);
+static void bbstreamer_gzip_extractor_finalize(bbstreamer *streamer);
+static void bbstreamer_gzip_extractor_free(bbstreamer *streamer);
+static void *gzip_palloc(void *opaque, unsigned items, unsigned size);
+static void gzip_pfree(void *opaque, void *address);
+
+const bbstreamer_ops bbstreamer_gzip_extractor_ops = {
+	.content = bbstreamer_gzip_extractor_content,
+	.finalize = bbstreamer_gzip_extractor_finalize,
+	.free = bbstreamer_gzip_extractor_free
+};
+#endif
+
+/*
+ * Create a bbstreamer that just compresses data using gzip, and then writes
+ * it to a file.
+ *
+ * As in the case of bbstreamer_plain_writer_new, pathname is always used
+ * for error reporting purposes; if file is NULL, it is also the opened and
+ * closed so that the data may be written there.
+ */
+bbstreamer *
+bbstreamer_gzip_writer_new(char *pathname, FILE *file, int compresslevel)
+{
+#ifdef HAVE_LIBZ
+	bbstreamer_gzip_writer *streamer;
+
+	streamer = palloc0(sizeof(bbstreamer_gzip_writer));
+	*((const bbstreamer_ops **) &streamer->base.bbs_ops) =
+		&bbstreamer_gzip_writer_ops;
+
+	streamer->pathname = pstrdup(pathname);
+
+	if (file == NULL)
+	{
+		streamer->gzfile = gzopen(pathname, "wb");
+		if (streamer->gzfile == NULL)
+		{
+			pg_log_error("could not create compressed file \"%s\": %m",
+						 pathname);
+			exit(1);
+		}
+	}
+	else
+	{
+		int			fd = dup(fileno(file));
+
+		if (fd < 0)
+		{
+			pg_log_error("could not duplicate stdout: %m");
+			exit(1);
+		}
+
+		streamer->gzfile = gzdopen(fd, "wb");
+		if (streamer->gzfile == NULL)
+		{
+			pg_log_error("could not open output file: %m");
+			exit(1);
+		}
+	}
+
+	if (gzsetparams(streamer->gzfile, compresslevel,
+					Z_DEFAULT_STRATEGY) != Z_OK)
+	{
+		pg_log_error("could not set compression level %d: %s",
+					 compresslevel, get_gz_error(streamer->gzfile));
+		exit(1);
+	}
+
+	return &streamer->base;
+#else
+	pg_log_error("this build does not support compression");
+	exit(1);
+#endif
+}
+
+#ifdef HAVE_LIBZ
+/*
+ * Write archive content to gzip file.
+ */
+static void
+bbstreamer_gzip_writer_content(bbstreamer *streamer,
+							   bbstreamer_member *member, const char *data,
+							   int len, bbstreamer_archive_context context)
+{
+	bbstreamer_gzip_writer *mystreamer;
+
+	mystreamer = (bbstreamer_gzip_writer *) streamer;
+
+	if (len == 0)
+		return;
+
+	errno = 0;
+	if (gzwrite(mystreamer->gzfile, data, len) != len)
+	{
+		/* if write didn't set errno, assume problem is no disk space */
+		if (errno == 0)
+			errno = ENOSPC;
+		pg_log_error("could not write to compressed file \"%s\": %s",
+					 mystreamer->pathname, get_gz_error(mystreamer->gzfile));
+		exit(1);
+	}
+}
+
+/*
+ * End-of-archive processing when writing to a gzip file consists of just
+ * calling gzclose.
+ *
+ * It makes no difference whether we opened the file or the caller did it,
+ * because libz provides no way of avoiding a close on the underling file
+ * handle. Notice, however, that bbstreamer_gzip_writer_new() uses dup() to
+ * work around this issue, so that the behavior from the caller's viewpoint
+ * is the same as for bbstreamer_plain_writer.
+ */
+static void
+bbstreamer_gzip_writer_finalize(bbstreamer *streamer)
+{
+	bbstreamer_gzip_writer *mystreamer;
+
+	mystreamer = (bbstreamer_gzip_writer *) streamer;
+
+	errno = 0;					/* in case gzclose() doesn't set it */
+	if (gzclose(mystreamer->gzfile) != 0)
+	{
+		pg_log_error("could not close compressed file \"%s\": %m",
+					 mystreamer->pathname);
+		exit(1);
+	}
+
+	mystreamer->gzfile = NULL;
+}
+
+/*
+ * Free memory associated with this bbstreamer.
+ */
+static void
+bbstreamer_gzip_writer_free(bbstreamer *streamer)
+{
+	bbstreamer_gzip_writer *mystreamer;
+
+	mystreamer = (bbstreamer_gzip_writer *) streamer;
+
+	Assert(mystreamer->base.bbs_next == NULL);
+	Assert(mystreamer->gzfile == NULL);
+
+	pfree(mystreamer->pathname);
+	pfree(mystreamer);
+}
+
+/*
+ * Helper function for libz error reporting.
+ */
+static const char *
+get_gz_error(gzFile gzf)
+{
+	int			errnum;
+	const char *errmsg;
+
+	errmsg = gzerror(gzf, &errnum);
+	if (errnum == Z_ERRNO)
+		return strerror(errno);
+	else
+		return errmsg;
+}
+#endif
+
+/*
+ * Create a new base backup streamer that performs decompression of gzip
+ * compressed blocks.
+ */
+bbstreamer *
+bbstreamer_gzip_extractor_new(bbstreamer *next)
+{
+#ifdef HAVE_LIBZ
+	bbstreamer_gzip_extractor	*streamer;
+	z_stream *zs;
+
+	Assert(next != NULL);
+
+	streamer = palloc0(sizeof(bbstreamer_gzip_extractor));
+	*((const bbstreamer_ops **) &streamer->base.bbs_ops) =
+		&bbstreamer_gzip_extractor_ops;
+
+	streamer->base.bbs_next = next;
+	initStringInfo(&streamer->base.bbs_buffer);
+
+	/* Initialize internal stream state for decompression */
+	zs = &streamer->zstream;
+	zs->zalloc = gzip_palloc;
+	zs->zfree = gzip_pfree;
+	zs->next_out = (uint8 *) streamer->base.bbs_buffer.data;
+	zs->avail_out = streamer->base.bbs_buffer.maxlen;
+
+	/*
+	 * Data compression was initialized using deflateInit2 to request a gzip
+	 * header. Similarly, we are using inflateInit2 to initialize data
+	 * decompression.
+	 *
+	 * Per the documentation of inflateInit2, the second argument is
+	 * "windowBits" and it's value must be greater than or equal to the value
+	 * provided while compressing the data, so we are using the maximum
+	 * possible value for safety.
+	 */
+	if (inflateInit2(zs, 15 + 16) != Z_OK)
+	{
+		pg_log_error("could not initialize compression library");
+		exit(1);
+
+	}
+
+	return &streamer->base;
+#else
+	pg_log_error("this build does not support compression");
+	exit(1);
+#endif
+}
+
+#ifdef HAVE_LIBZ
+/*
+ * Decompress the input data to output buffer until we ran out of the input
+ * data. Each time the output buffer is full invoke bbstreamer_content to pass
+ * on the decompressed data to next streamer.
+ */
+static void
+bbstreamer_gzip_extractor_content(bbstreamer *streamer,
+								  bbstreamer_member *member,
+								  const char *data, int len,
+								  bbstreamer_archive_context context)
+{
+	bbstreamer_gzip_extractor *mystreamer = (bbstreamer_gzip_extractor *) streamer;
+	z_stream *zs = &mystreamer->zstream;
+
+
+	zs->next_in = (uint8 *) data;
+	zs->avail_in = len;
+
+	/* Process the current chunk */
+	while (zs->avail_in > 0)
+	{
+		int res;
+
+		Assert(mystreamer->bytes_written < mystreamer->base.bbs_buffer.maxlen);
+
+		zs->next_out = (uint8 *)
+			mystreamer->base.bbs_buffer.data + mystreamer->bytes_written;
+		zs->avail_out = mystreamer->base.bbs_buffer.maxlen - mystreamer->bytes_written;
+
+		/*
+		 * Decompresses data starting at zs->next_in and update zs->next_in
+		 * and zs->avail_in, generate output data starting at zs->next_out
+		 * and update zs->next_out and zs->avail_out accordingly.
+		 */
+		res = inflate(zs, Z_NO_FLUSH);
+
+		if (res == Z_STREAM_ERROR)
+			pg_log_error("could not decompress data: %s", zs->msg);
+
+		mystreamer->bytes_written = mystreamer->base.bbs_buffer.maxlen - zs->avail_out;
+
+		/* If output buffer is full then pass on the content to next streamer */
+		if (mystreamer->bytes_written >= mystreamer->base.bbs_buffer.maxlen)
+		{
+			bbstreamer_content(mystreamer->base.bbs_next, member,
+							   mystreamer->base.bbs_buffer.data,
+							   mystreamer->base.bbs_buffer.maxlen, context);
+			mystreamer->bytes_written = 0;
+		}
+	}
+}
+
+/*
+ * End-of-stream processing.
+ */
+static void
+bbstreamer_gzip_extractor_finalize(bbstreamer *streamer)
+{
+	bbstreamer_gzip_extractor *mystreamer = (bbstreamer_gzip_extractor *) streamer;
+
+	/*
+	 * End of the stream, if there is some pending data in output buffers then
+	 * we must forward it to next streamer.
+	 */
+	bbstreamer_content(mystreamer->base.bbs_next, NULL,
+					   mystreamer->base.bbs_buffer.data,
+					   mystreamer->base.bbs_buffer.maxlen,
+					   BBSTREAMER_UNKNOWN);
+
+	bbstreamer_finalize(mystreamer->base.bbs_next);
+}
+
+/*
+ * Free memory.
+ */
+static void
+bbstreamer_gzip_extractor_free(bbstreamer *streamer)
+{
+	bbstreamer_gzip_extractor *mystreamer = (bbstreamer_gzip_extractor *) streamer;
+
+	bbstreamer_free(mystreamer->base.bbs_next);
+	pfree(mystreamer->base.bbs_buffer.data);
+	pfree(streamer);
+}
+
+/*
+ * Wrapper function to adjust the signature of palloc to match what libz
+ * expects.
+ */
+static void *
+gzip_palloc(void *opaque, unsigned items, unsigned size)
+{
+	return palloc(items * size);
+}
+
+/*
+ * Wrapper function to adjust the signature of pfree to match what libz
+ * expects.
+ */
+static void
+gzip_pfree(void *opaque, void *address)
+{
+	pfree(address);
+}
+#endif
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 6ee49a5..d43eb4b 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -111,6 +111,12 @@ typedef enum
 	STREAM_WAL
 } IncludeWal;
 
+typedef enum
+{
+	BACKUP_COMPRESSION_NONE,
+	BACKUP_COMPRESSION_GZIP
+} compression_type;
+
 /* Global options */
 static char *basedir = NULL;
 static TablespaceList tablespace_dirs = {NULL, NULL};
@@ -173,6 +179,10 @@ static int	has_xlogendptr = 0;
 static volatile LONG has_xlogendptr = 0;
 #endif
 
+/* Server side compression method and compression level */
+static compression_type	server_compression_type = BACKUP_COMPRESSION_NONE;
+static int				server_compression_level = 0;
+
 /* Contents of configuration file to be generated */
 static PQExpBuffer recoveryconfcontents = NULL;
 
@@ -1002,7 +1012,8 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 	bbstreamer *streamer;
 	bbstreamer *manifest_inject_streamer = NULL;
 	bool		inject_manifest;
-	bool		is_tar;
+	bool		is_tar,
+				is_tar_gz;
 	bool		must_parse_archive;
 	int			archive_name_len = strlen(archive_name);
 
@@ -1017,6 +1028,10 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 	is_tar = (archive_name_len > 4 &&
 			  strcmp(archive_name + archive_name_len - 4, ".tar") == 0);
 
+	/* Is this a gzip archive? */
+	is_tar_gz = (archive_name_len > 8 &&
+				 strcmp(archive_name + archive_name_len - 3, ".gz") == 0);
+
 	/*
 	 * We have to parse the archive if (1) we're suppose to extract it, or if
 	 * (2) we need to inject backup_manifest or recovery configuration into it.
@@ -1025,8 +1040,8 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 	must_parse_archive = (format == 'p' || inject_manifest ||
 		(spclocation == NULL && writerecoveryconf));
 
-	/* At present, we only know how to parse tar archives. */
-	if (must_parse_archive && !is_tar)
+	/* At present, we only know how to parse tar and gzip archives. */
+	if (must_parse_archive && !is_tar && !is_tar_gz)
 	{
 		pg_log_error("unable to parse archive: %s", archive_name);
 		pg_log_info("only tar archives can be parsed");
@@ -1136,6 +1151,13 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 	else if (expect_unterminated_tarfile)
 		streamer = bbstreamer_tar_terminator_new(streamer);
 
+	/*
+	 * Extract the gzip compressed archive using a gzip extractor and then
+	 * forward it to next streamer.
+	 */
+	if (format == 'p' && server_compression_type == BACKUP_COMPRESSION_GZIP)
+		streamer = bbstreamer_gzip_extractor_new(streamer);
+
 	/* Return the results. */
 	*manifest_inject_streamer_p = manifest_inject_streamer;
 	return streamer;
@@ -2448,6 +2470,21 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	if (server_compression != NULL)
+	{
+		if (strcmp(server_compression, "gzip") == 0)
+			server_compression_type = BACKUP_COMPRESSION_GZIP;
+		else if (strlen(server_compression) == 5 &&
+				strncmp(server_compression, "gzip", 4) == 0 &&
+				server_compression[4] >= '1' && server_compression[4] <= '9')
+		{
+			server_compression_type = BACKUP_COMPRESSION_GZIP;
+			server_compression_level = server_compression[4] - '0';
+		}
+	}
+	else
+		server_compression_type = BACKUP_COMPRESSION_NONE;
+
 	/*
 	 * Compression doesn't make sense unless tar format is in use.
 	 */
-- 
1.8.3.1

From b54f40721fedb566cd212061fd2a10fe50c31a5a Mon Sep 17 00:00:00 2001
From: Dipesh Pandit <[email-masked]>
Date: Thu, 20 Jan 2022 17:44:52 +0530
Subject: [PATCH 2/2] Test plain format server compressed gzip backup

---
 src/bin/pg_verifybackup/t/008_untar.pl | 111 ++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 37 deletions(-)
 mode change 100644 => 100755 src/bin/pg_verifybackup/t/008_untar.pl

diff --git a/src/bin/pg_verifybackup/t/008_untar.pl b/src/bin/pg_verifybackup/t/008_untar.pl
old mode 100644
new mode 100755
index 85946cf..0885c5c
--- a/src/bin/pg_verifybackup/t/008_untar.pl
+++ b/src/bin/pg_verifybackup/t/008_untar.pl
@@ -11,7 +11,7 @@ use Config;
 use File::Path qw(rmtree);
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 6;
+use Test::More tests => 10;
 
 my $primary = PostgreSQL::Test::Cluster->new('primary');
 $primary->init(allows_streaming => 1);
@@ -35,6 +35,12 @@ my @test_configuration = (
 		'decompress_program' => $ENV{'GZIP_PROGRAM'},
 		'decompress_flags' => [ '-d' ],
 		'enabled' => check_pg_config("#define HAVE_LIBZ 1")
+	},
+	{
+		'compression_method' => 'gzip',
+		'backup_flags' => ['--server-compress', 'gzip', '-Fp'],
+		'plain_format' => 1,
+		'enabled' => check_pg_config("#define HAVE_LIBZ 1"),
 	}
 );
 
@@ -51,54 +57,85 @@ for my $tc (@test_configuration)
 
 		# Take a server-side backup.
 		my @backup = (
-			'pg_basebackup', '--no-sync', '-cfast', '--target',
-			"server:$backup_path", '-Xfetch'
+			'pg_basebackup', '--no-sync', '-cfast', '-Xfetch'
 		);
+
+		if (! $tc->{'plain_format'})
+		{
+			push @backup, '--target', "server:$backup_path";
+		}
+		else
+		{
+			# Target cannot be used with plain format backup.
+			push @backup, '-D', "$backup_path";
+
+			# Make sure that backup directory is empty.
+			rmtree($backup_path);
+		}
+
 		push @backup, @{$tc->{'backup_flags'}};
 		$primary->command_ok(\@backup,
 							 "server side backup, compression $method");

-		# Verify that the we got the files we expected.
-		my $backup_files = join(',',
-			sort grep { $_ ne '.' && $_ ne '..' } slurp_dir($backup_path));
-		my $expected_backup_files = join(',',
-			sort ('backup_manifest', $tc->{'backup_archive'}));
-		is($backup_files,$expected_backup_files,
-			"found expected backup files, compression $method");
-
-		# Decompress.
-		if (exists $tc->{'decompress_program'})
+		if (! $tc->{'plain_format'})
 		{
-			my @decompress = ($tc->{'decompress_program'});
-			push @decompress, @{$tc->{'decompress_flags'}}
-				if $tc->{'decompress_flags'};
-			push @decompress, $backup_path . '/' . $tc->{'backup_archive'};
-			system_or_bail(@decompress);
-		}
+			# Verify that the we got the files we expected.
+			my $backup_files = join(',',
+				sort grep { $_ ne '.' && $_ ne '..' } slurp_dir($backup_path));
+			my $expected_backup_files = join(',',
+				sort ('backup_manifest', $tc->{'backup_archive'}));
+			is($backup_files,$expected_backup_files,
+				"found expected backup files, compression $method");
+
+			# Decompress.
+			if (exists $tc->{'decompress_program'})
+			{
+				my @decompress = ($tc->{'decompress_program'});
+				push @decompress, @{$tc->{'decompress_flags'}}
+					if $tc->{'decompress_flags'};
+				push @decompress, $backup_path . '/' . $tc->{'backup_archive'};
+				system_or_bail(@decompress);
+			}
+
+			SKIP: {
+				my $tar = $ENV{TAR};
+				# don't check for a working tar here, to accommodate various odd
+				# cases such as AIX. If tar doesn't work the init_from_backup below
+				# will fail.
+				skip "no tar program available", 1
+					if (!defined $tar || $tar eq '');
 
-		SKIP: {
-			my $tar = $ENV{TAR};
-			# don't check for a working tar here, to accomodate various odd
-			# cases such as AIX. If tar doesn't work the init_from_backup below
-			# will fail.
-			skip "no tar program available", 1
-				if (!defined $tar || $tar eq '');
+				# Untar.
+				mkdir($extract_path);
+				system_or_bail($tar, 'xf', $backup_path . '/base.tar',
+					'-C', $extract_path);
 
-			# Untar.
-			mkdir($extract_path);
-			system_or_bail($tar, 'xf', $backup_path . '/base.tar',
-				'-C', $extract_path);
+				# Verify.
+				$primary->command_ok([ 'pg_verifybackup', '-n',
+						'-m', "$backup_path/backup_manifest", '-e', $extract_path ],
+					    "verify backup, compression $method");
+			}
 
-			# Verify.
+			# Cleanup.
+			unlink($backup_path . '/backup_manifest');
+			unlink($backup_path . '/base.tar');
+			rmtree($extract_path);
+		}
+		else
+		{
+			# Verify that the we got the files we expected.
+			ok (-f "$backup_path/PG_VERSION", "backup with plain format created");
+			ok (-f "$backup_path/backup_manifest", "backup manifest included");
+
+			# Verify plain format backup with server compression
 			$primary->command_ok([ 'pg_verifybackup', '-n',
-				'-m', "$backup_path/backup_manifest", '-e', $extract_path ],
-				"verify backup, compression $method");
+					'-m', "$backup_path/backup_manifest", '-e', $backup_path ],
+					"verify plain format backup, compression $method");
+
+			# Cleanup.
+			rmtree($backup_path);
 		}
 
-		# Cleanup.
-		unlink($backup_path . '/backup_manifest');
-		unlink($backup_path . '/base.tar');
-		rmtree($extract_path);
 	}
 }
-- 
1.8.3.1

---

From: Robert Haas <[email-masked]>
Date: Thu, 20 Jan 2022 11:10:01 -0500

> Thanks for the feedback, I have incorporated the suggestions and

Cool. I'll do a detailed review later, but I think this is going in a
good direction.

> this using pg_verifybackup. I have updated (v2-0002) for the test

Interesting approach. This unfortunately has the effect of making that
test case file look a bit incoherent -- the comment at the top of the
file isn't really accurate any more, for example, and the plain_format
flag does more than just cause us to use -Fp; it also causes us NOT to
use --target server:X. However, that might be something we can figure
out a way to clean up. Alternatively, we could have a new test case
file that is structured like 002_algorithm.pl but looping over
compression methods rather than checksum algorithms, and testing each
one with --server-compress and -Fp. It might be easier to make that
look nice (but I'm not 100% sure).

-- 
Robert Haas
EDB: http://www.enterprisedb.com