Fix printing last progress report line in client programs.

hlinnaka · hlinnaka · commit 9f44d71b06b0 · 2020-08-17T09:33:10.000+03:00
A number of client programs have a "--progress" option that when printing to a TTY, updates the current line by printing a '\r' and overwriting it. After the last line, '\n' needs to be printed to move the cursor to the next line. pg_basebackup and pgbench got this right, but pg_rewind and pg_checksums were slightly wrong. pg_rewind printed the newline to stdout instead of stderr, and pg_checksums printed the newline even when not printing to a TTY. Fix them, and also add a 'finished' argument to pg_basebackup's progress_report() function, to keep it consistent with the other programs. Backpatch to v12. pg_rewind's newline was broken with the logging changes in commit cc8d415 in v12, and pg_checksums was introduced in v12. Discussion: https://www.postgresql.org/message-id/82b539e5-ae33-34b0-1aee-22b3379fd3eb@iki.fi
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
@@ -143,7 +143,8 @@ static PQExpBuffer recoveryconfcontents = NULL;
 /* Function headers */
 static void usage(void);
 static void verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found);
-static void progress_report(int tablespacenum, const char *filename, bool force);
+static void progress_report(int tablespacenum, const char *filename, bool force,
+							bool finished);
 
 static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
 static void ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum);
@@ -706,11 +707,15 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found)
  * Print a progress report based on the global variables. If verbose output
  * is enabled, also print the current file name.
  *
- * Progress report is written at maximum once per second, unless the
- * force parameter is set to true.
+ * Progress report is written at maximum once per second, unless the force
+ * parameter is set to true.
+ *
+ * If finished is set to true, this is the last progress report. The cursor
+ * is moved to the next line.
  */
 static void
-progress_report(int tablespacenum, const char *filename, bool force)
+progress_report(int tablespacenum, const char *filename,
+				bool force, bool finished)
 {
 	int			percent;
 	char		totaldone_str[32];
@@ -721,7 +726,7 @@ progress_report(int tablespacenum, const char *filename, bool force)
 		return;
 
 	now = time(NULL);
-	if (now == last_progress_report && !force)
+	if (now == last_progress_report && !force && !finished)
 		return;					/* Max once per second */
 
 	last_progress_report = now;
@@ -792,10 +797,11 @@ progress_report(int tablespacenum, const char *filename, bool force)
 				totaldone_str, totalsize_str, percent,
 				tablespacenum, tablespacecount);
 
-	if (isatty(fileno(stderr)))
-		fprintf(stderr, "\r");
-	else
-		fprintf(stderr, "\n");
+	/*
+	 * Stay on the same line if reporting to a terminal and we're not done
+	 * yet.
+	 */
+	fprintf(stderr, (!finished && isatty(fileno(stderr))) ? "\r" : "\n");
 }
 
 static int32
@@ -1355,9 +1361,9 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 			}
 		}
 		totaldone += r;
-		progress_report(rownum, filename, false);
+		progress_report(rownum, filename, false, false);
 	}							/* while (1) */
-	progress_report(rownum, filename, true);
+	progress_report(rownum, filename, true, false);
 
 	if (copybuf != NULL)
 		PQfreemem(copybuf);
@@ -1614,7 +1620,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 				exit(1);
 			}
 			totaldone += r;
-			progress_report(rownum, filename, false);
+			progress_report(rownum, filename, false, false);
 
 			current_len_left -= r;
 			if (current_len_left == 0 && current_padding == 0)
@@ -1630,7 +1636,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 			}
 		}						/* continuing data in existing file */
 	}							/* loop over all data blocks */
-	progress_report(rownum, filename, true);
+	progress_report(rownum, filename, true, false);
 
 	if (file != NULL)
 	{
@@ -2011,11 +2017,7 @@ BaseBackup(void)
 	}							/* Loop over all tablespaces */
 
 	if (showprogress)
-	{
-		progress_report(PQntuples(res), NULL, true);
-		if (isatty(fileno(stderr)))
-			fprintf(stderr, "\n");	/* Need to move to next line */
-	}
+		progress_report(PQntuples(res), NULL, true, true);
 
 	PQclear(res);
 
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
@@ -124,7 +124,7 @@ static const struct exclude_list_item skip[] = {
  * src/bin/pg_basebackup/pg_basebackup.c.
  */
 static void
-progress_report(bool force)
+progress_report(bool finished)
 {
 	int			percent;
 	char		total_size_str[32];
@@ -134,7 +134,7 @@ progress_report(bool force)
 	Assert(showprogress);
 
 	now = time(NULL);
-	if (now == last_progress_report && !force)
+	if (now == last_progress_report && !finished)
 		return;					/* Max once per second */
 
 	/* Save current time */
@@ -161,8 +161,11 @@ progress_report(bool force)
 			(int) strlen(current_size_str), current_size_str, total_size_str,
 			percent);
 
-	/* Stay on the same line if reporting to a terminal */
-	fprintf(stderr, isatty(fileno(stderr)) ? "\r" : "\n");
+	/*
+	 * Stay on the same line if reporting to a terminal and we're not done
+	 * yet.
+	 */
+	fprintf(stderr, (!finished && isatty(fileno(stderr))) ? "\r" : "\n");
 }
 
 static bool
@@ -623,10 +626,7 @@ main(int argc, char *argv[])
 		(void) scan_directory(DataDir, "pg_tblspc", false);
 
 		if (showprogress)
-		{
 			progress_report(true);
-			fprintf(stderr, "\n");	/* Need to move to next line */
-		}
 
 		printf(_("Checksum operation completed\n"));
 		printf(_("Files scanned:  %s\n"), psprintf(INT64_FORMAT, files));
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
@@ -359,7 +359,6 @@ main(int argc, char **argv)
 	executeFileMap();
 
 	progress_report(true);
-	printf("\n");
 
 	if (showprogress)
 		pg_log_info("creating backup label and updating control file");
@@ -452,11 +451,14 @@ sanityChecks(void)
 /*
  * Print a progress report based on the fetch_size and fetch_done variables.
  *
- * Progress report is written at maximum once per second, unless the
- * force parameter is set to true.
+ * Progress report is written at maximum once per second, except that the
+ * last progress report is always printed.
+ *
+ * If finished is set to true, this is the last progress report. The cursor
+ * is moved to the next line.
  */
 void
-progress_report(bool force)
+progress_report(bool finished)
 {
 	static pg_time_t last_progress_report = 0;
 	int			percent;
@@ -468,7 +470,7 @@ progress_report(bool force)
 		return;
 
 	now = time(NULL);
-	if (now == last_progress_report && !force)
+	if (now == last_progress_report && !finished)
 		return;					/* Max once per second */
 
 	last_progress_report = now;
@@ -498,10 +500,12 @@ progress_report(bool force)
 	fprintf(stderr, _("%*s/%s kB (%d%%) copied"),
 			(int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
 			percent);
-	if (isatty(fileno(stderr)))
-		fprintf(stderr, "\r");
-	else
-		fprintf(stderr, "\n");
+
+	/*
+	 * Stay on the same line if reporting to a terminal and we're not done
+	 * yet.
+	 */
+	fprintf(stderr, (!finished && isatty(fileno(stderr))) ? "\r" : "\n");
 }
 
 /*
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
@@ -49,7 +49,7 @@ extern XLogRecPtr readOneRecord(const char *datadir, XLogRecPtr ptr,
 								int tliIndex);
 
 /* in pg_rewind.c */
-extern void progress_report(bool force);
+extern void progress_report(bool finished);
 
 /* in timeline.c */
 extern TimeLineHistoryEntry *rewind_parseTimeLineHistory(char *buffer,
