From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_basebackup's --gzip switch misbehaves
Date: Tue, 13 Sep 2022 17:38:47 -0400
Message-ID: <2402847.1663105127@sss.pgh.pa.us> (raw)
In-Reply-To: <YyAtkOU/C/SD9qlL@paquier.xyz>
References: <1400032.1662217889@sss.pgh.pa.us>
<YxQ1tIYF8hbrNO7q@paquier.xyz>
<YyAtkOU/C/SD9qlL@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
> Attached is the patch I am finishing with, consisting of:
> - the removal of PG_COMPRESSION_OPTION_LEVEL.
> - assigning a default compression level when nothing is specified in
> the spec.
> - a couple of complifications in pg_receivewal, pg_basebackup and the
> backend code as there is no need to worry about the compression
> level.
This looks good to me. It seems simpler, and I concur that it
fixes the described problem. I now see
tmp_check/backup_gzip:
total 3668
-rw-r-----. 1 postgres postgres 137756 Sep 13 17:29 backup_manifest
-rw-r-----. 1 postgres postgres 3537499 Sep 13 17:29 base.tar.gz
-rw-r-----. 1 postgres postgres 73989 Sep 13 17:29 pg_wal.tar.gz
tmp_check/backup_gzip2:
total 3168
-rw-r-----. 1 postgres postgres 137756 Sep 13 17:29 backup_manifest
-rw-r-----. 1 postgres postgres 3083516 Sep 13 17:29 base.tar.gz
-rw-r-----. 1 postgres postgres 17069 Sep 13 17:29 pg_wal.tar.gz
tmp_check/backup_gzip3:
total 3668
-rw-r-----. 1 postgres postgres 137756 Sep 13 17:29 backup_manifest
-rw-r-----. 1 postgres postgres 3537517 Sep 13 17:29 base.tar.gz
-rw-r-----. 1 postgres postgres 73988 Sep 13 17:29 pg_wal.tar.gz
which looks sane: the gzip2 case should, and does, have better
compression than the other two.
BTW, this bit:
inline patch 1: src/bin/pg_basebackup/t/010_pg_basebackup.pl (+0/-3 lines) download inline-1.patchdiff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.plindex 3d1a4ddd5c..40f1d3f7e2 100644--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl@@ -860,9 +860,6 @@ SKIP:
my $gzip_is_valid =
system_log($gzip, '--test', @zlib_files, @zlib_files2, @zlib_files3);
is($gzip_is_valid, 0, "gzip verified the integrity of compressed data");
- rmtree("$tempdir/backup_gzip");- rmtree("$tempdir/backup_gzip2");- rmtree("$tempdir/backup_gzip3");
}
# Test background stream process terminating before the basebackup has
is something I tried along the way to diagnosing the problem, and
it turns out to have exactly zero effect. The $tempdir is some
temporary subdirectory of tmp_check that will get nuked at the end
of the TAP test no matter what. So these rmtrees are merely making
the evidence disappear a bit faster; it will anyway.
What I did to diagnose the problem was this:
@@ -860,9 +860,9 @@ SKIP:
my $gzip_is_valid =
system_log($gzip, '--test', @zlib_files, @zlib_files2, @zlib_files3);
is($gzip_is_valid, 0, "gzip verified the integrity of compressed data");
- rmtree("$tempdir/backup_gzip");
- rmtree("$tempdir/backup_gzip2");
- rmtree("$tempdir/backup_gzip3");
+ system_log('mv', "$tempdir/backup_gzip", "tmp_check");
+ system_log('mv', "$tempdir/backup_gzip2", "tmp_check");
+ system_log('mv', "$tempdir/backup_gzip3", "tmp_check");
}
# Test background stream process terminating before the basebackup has
which is not real clean, since then the files get left behind even
on success, which I doubt we want either.
Anyway, I have no objection to dropping the rmtrees, since they're
pretty useless as the code stands. Just wanted to mention this
issue for the archives.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_basebackup's --gzip switch misbehaves
In-Reply-To: <2402847.1663105127@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox