From 542d7817f774ea9d94798eb95cdf250d4f1527d9 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 16 Apr 2020 13:57:07 +0900 Subject: [PATCH] Disable silently generation of manifests with servers <= 12 in pg_basebackup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 0d8c9c1, pg_basebackup would generate an error if connected to a backend version older than 12 where backup manifests are not supported. Avoiding this error is possible by using the --no-manifest option. This error handling could be confusing for some users, where patching a backup script that interacts with multiple backend versions would cause the addition of --no-manifest to potentially not generate a backup manifest even for Postgres 13 and newer versions. As we want to encourage the use of backup manifests as much as possible, this commit silently disables manifests where not supported, instead of generating an error. While on it, rework a bit the code to make it more consistent with the surroundings when generating the BASE_BACKUP command. Per discussion with Andres Freund, Stephen Frost, Robert Haas, Álvaro Herrera, Kyotaro Horiguchi, Tom Lane, David Steele, and me. Author: Michael Paquier Discussion: https://postgr.es/m/20200410080910.GZ1606@paquier.xyz --- src/bin/pg_basebackup/pg_basebackup.c | 29 ++++++++++----------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 65ca1b16f0..faa69d14ce 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -108,6 +108,11 @@ typedef void (*WriteDataCallback) (size_t nbytes, char *buf, */ #define MINIMUM_VERSION_FOR_TEMP_SLOTS 100000 +/* + * Backup manifests are supported from version 13. + */ +#define MINIMUM_VERSION_FOR_MANIFESTS 130000 + /* * Different ways to include WAL */ @@ -1770,7 +1775,7 @@ BaseBackup(void) char *basebkp; char escaped_label[MAXPGPATH]; char *maxrate_clause = NULL; - char *manifest_clause; + char *manifest_clause = NULL; char *manifest_checksums_clause = ""; int i; char xlogstart[64]; @@ -1836,15 +1841,6 @@ BaseBackup(void) if (manifest) { - if (serverMajor < 1300) - { - const char *serverver = PQparameterStatus(conn, "server_version"); - - pg_log_error("backup manifests are not supported by server version %s", - serverver ? serverver : "'unknown'"); - exit(1); - } - if (manifest_force_encode) manifest_clause = "MANIFEST 'force-encode'"; else @@ -1853,13 +1849,6 @@ BaseBackup(void) manifest_checksums_clause = psprintf("MANIFEST_CHECKSUMS '%s'", manifest_checksums); } - else - { - if (serverMajor < 1300) - manifest_clause = ""; - else - manifest_clause = "MANIFEST 'no'"; - } if (verbose) pg_log_info("initiating base backup, waiting for checkpoint to complete"); @@ -1883,7 +1872,7 @@ BaseBackup(void) maxrate_clause ? maxrate_clause : "", format == 't' ? "TABLESPACE_MAP" : "", verify_checksums ? "" : "NOVERIFY_CHECKSUMS", - manifest_clause, + manifest_clause ? manifest_clause : "", manifest_checksums_clause); if (PQsendQuery(conn, basebkp) == 0) @@ -2589,6 +2578,10 @@ main(int argc, char **argv) */ umask(pg_mode_mask); + /* Backup manifests are supported in 13 and newer versions */ + if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_MANIFESTS) + manifest = false; + /* * Verify that the target directory exists, or create it. For plaintext * backups, always require the directory. For tar backups, require it