From 9f44d71b06b0d457ad3e5d7fdcbba2039f3000ef Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 17 Aug 2020 09:27:29 +0300 Subject: [PATCH] Fix printing last progress report line in client programs. 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 cc8d415117 in v12, and pg_checksums was introduced in v12. Discussion: https://www.postgresql.org/message-id/82b539e5-ae33-34b0-1aee-22b3379fd3eb@iki.fi --- src/bin/pg_basebackup/pg_basebackup.c | 38 ++++++++++++++------------- src/bin/pg_checksums/pg_checksums.c | 14 +++++----- src/bin/pg_rewind/pg_rewind.c | 22 +++++++++------- src/bin/pg_rewind/pg_rewind.h | 2 +- 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index d472f4d1c23..54bf864efde 100644 --- 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 index 03c3da3d730..c4db327a673 100644 --- 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 index f9d94764ad6..59c8695b6bd 100644 --- 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 index 1125c7e60f6..235f9c46b12 100644 --- 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,