Improve base backup protocol documentation

Started by Peter Eisentrautover 6 years ago3 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

It was apparently entirely undocumented that the tablespace size
estimates sent by the base backup protocol are in kilobytes. Here is a
patch to document that. Also, a related clarification in the
pg_basebackup.c source code: It was not clear without analyzing the
whole stack that "totalsize" is in kilobytes and "totaldone" is in bytes.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Improve-base-backup-protocol-documentation.patchtext/plain; charset=UTF-8; name=0001-Improve-base-backup-protocol-documentation.patch; x-mac-creator=0; x-mac-type=0Download
From 2b9c1b1c8397bf9f6613d0d134ad3fa19a9292b9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 28 Aug 2019 16:46:54 +0200
Subject: [PATCH] Improve base backup protocol documentation

Document that the tablespace sizes are in units of kilobytes.  Make
the pg_basebackup source code a bit clearer about this, too.
---
 doc/src/sgml/protocol.sgml            |  4 ++--
 src/bin/pg_basebackup/pg_basebackup.c | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index b20f1690a7..f036d5f178 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2615,8 +2615,8 @@ <title>Streaming Replication Protocol</title>
         <term><literal>size</literal> (<type>int8</type>)</term>
         <listitem>
          <para>
-          The approximate size of the tablespace, if progress report has
-          been requested; otherwise it's null.
+          The approximate size of the tablespace, in kilobytes (1024 bytes),
+          if progress report has been requested; otherwise it's null.
          </para>
         </listitem>
        </varlistentry>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 9207109ba3..498754eb32 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -115,7 +115,7 @@ static bool made_tablespace_dirs = false;
 static bool found_tablespace_dirs = false;
 
 /* Progress counters */
-static uint64 totalsize;
+static uint64 totalsize_kb;
 static uint64 totaldone;
 static int	tablespacecount;
 
@@ -722,7 +722,7 @@ progress_report(int tablespacenum, const char *filename, bool force)
 		return;					/* Max once per second */
 
 	last_progress_report = now;
-	percent = totalsize ? (int) ((totaldone / 1024) * 100 / totalsize) : 0;
+	percent = totalsize_kb ? (int) ((totaldone / 1024) * 100 / totalsize_kb) : 0;
 
 	/*
 	 * Avoid overflowing past 100% or the full size. This may make the total
@@ -732,8 +732,8 @@ progress_report(int tablespacenum, const char *filename, bool force)
 	 */
 	if (percent > 100)
 		percent = 100;
-	if (totaldone / 1024 > totalsize)
-		totalsize = totaldone / 1024;
+	if (totaldone / 1024 > totalsize_kb)
+		totalsize_kb = totaldone / 1024;
 
 	/*
 	 * Separate step to keep platform-dependent format code out of
@@ -742,7 +742,7 @@ progress_report(int tablespacenum, const char *filename, bool force)
 	 */
 	snprintf(totaldone_str, sizeof(totaldone_str), INT64_FORMAT,
 			 totaldone / 1024);
-	snprintf(totalsize_str, sizeof(totalsize_str), INT64_FORMAT, totalsize);
+	snprintf(totalsize_str, sizeof(totalsize_str), INT64_FORMAT, totalsize_kb);
 
 #define VERBOSE_FILENAME_LENGTH 35
 	if (verbose)
@@ -1942,11 +1942,11 @@ BaseBackup(void)
 	/*
 	 * Sum up the total size, for progress reporting
 	 */
-	totalsize = totaldone = 0;
+	totalsize_kb = totaldone = 0;
 	tablespacecount = PQntuples(res);
 	for (i = 0; i < PQntuples(res); i++)
 	{
-		totalsize += atol(PQgetvalue(res, i, 2));
+		totalsize_kb += atol(PQgetvalue(res, i, 2));
 
 		/*
 		 * Verify tablespace directories are empty. Don't bother with the
-- 
2.22.0

#2Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#1)
Re: Improve base backup protocol documentation

On Wed, Aug 28, 2019 at 4:58 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

It was apparently entirely undocumented that the tablespace size
estimates sent by the base backup protocol are in kilobytes. Here is a
patch to document that. Also, a related clarification in the
pg_basebackup.c source code: It was not clear without analyzing the
whole stack that "totalsize" is in kilobytes and "totaldone" is in bytes.

+1, these both look like reasonable changes to me.

//Magnus

#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Magnus Hagander (#2)
Re: Improve base backup protocol documentation

On 2019-09-03 09:50, Magnus Hagander wrote:

On Wed, Aug 28, 2019 at 4:58 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:

It was apparently entirely undocumented that the tablespace size
estimates sent by the base backup protocol are in kilobytes.  Here is a
patch to document that.  Also, a related clarification in the
pg_basebackup.c source code: It was not clear without analyzing the
whole stack that "totalsize" is in kilobytes and "totaldone" is in
bytes.

+1, these both look like reasonable changes to me.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services