Re: pg_basebackup, manifests and backends older than ~12

[Michael Paquier]

On Sun, Apr 12, 2020 at 08:08:17AM +0900, Michael Paquier wrote:
> Exactly.  My point is exactly that.  The current code would force
> users maintaining scripts with pg_basebackup to use --no-manifest if
> such a script runs with older versions of Postgres, but we should
> encourage users not do to that because we want them to use manifests
> with backend versions where they are supported.

Please note that I have added an open item for this thread, and
attached is a proposal of patch.  While reading the code, I have
noticed that the minimum version handling is not consistent with the
other MINIMUM_VERSION_*, so I have added one for manifests.
--
Michael

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index de098b3558..9e5e96bd8b 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

signature.asc
Description: PGP signature