On 2024/11/07 10:41, Tatsuo Ishii wrote:
The patch works perfectly for the case that there is one extra brace
as shown in your example. However I think it will not work if there
are two or more extra braces.

Are you suggesting adding more space characters before the carriage
return
in the progress reporting line, like this?

No. I thought about dynamically calculating spaces needed to be
printed something like attached patch.

Since the line includes
both
elapsed and remaining times, its total length doesn’t change much.

Maybe. But I am also worried about the case when we would want to
change the log line format in the future. We might introduce this kind
of bug again.  By dynamically calculating the number of necessary
spaces, we don't need to think about the concern.

+1

+                       if (previous_chars != 0)
+                       {
+                               int     n_spaces = chars - previous_chars;
+                               fprintf(stderr, "%*s", n_spaces, "");
+                               chars += n_spaces;
+                       }

Currently, this is added only when use_quiet is false, but shouldn’t it also 
apply when use_quiet is true?

Also, what happens if chars is smaller than previous_chars?

I’m unsure why chars needs to be incremented by n_spaces.


I’ve created an updated patch based on your idea—could you take a look?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 7f0d10e9c2c65527fbb01acd777366d00596e72c Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Fri, 8 Nov 2024 00:34:55 +0900
Subject: [PATCH v3] pgbench: Ensure previous progress message is fully cleared
 when updating.

During pgbench's table initialization, progress updates could display
leftover characters from the previous message if the new message
was shorter. This commit resolves the issue by appending spaces to
the current message to fully overwrite any remaining characters from
the previous line.

Back-patch to all the supported versions.

Author: Yushi Ogiwara, Tatsuo Ishii, Fujii Masao
Reviewed-by: Tatsuo Ishii, Fujii Masao
Discussion: https://postgr.es/m/9a9b8b95b6a709877ae48ad5b0c59...@oss.nttdata.com
---
 src/bin/pgbench/pgbench.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ba247f3f85..19a82244b5 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -4944,6 +4944,7 @@ initPopulateTable(PGconn *con, const char *table, int64 
base,
        int                     n;
        int64           k;
        int                     chars = 0;
+       int                     prev_chars = 0;
        PGresult   *res;
        PQExpBufferData sql;
        char            copy_statement[256];
@@ -5004,10 +5005,10 @@ initPopulateTable(PGconn *con, const char *table, int64 
base,
                        double          elapsed_sec = 
PG_TIME_GET_DOUBLE(pg_time_now() - start);
                        double          remaining_sec = ((double) total - j) * 
elapsed_sec / j;
 
-                       chars = fprintf(stderr, INT64_FORMAT " of " 
INT64_FORMAT " tuples (%d%%) of %s done (elapsed %.2f s, remaining %.2f s)%c",
+                       chars = fprintf(stderr, INT64_FORMAT " of " 
INT64_FORMAT " tuples (%d%%) of %s done (elapsed %.2f s, remaining %.2f s)",
                                                        j, total,
                                                        (int) ((j * 100) / 
total),
-                                                       table, elapsed_sec, 
remaining_sec, eol);
+                                                       table, elapsed_sec, 
remaining_sec);
                }
                /* let's not call the timing for each row, but only each 100 
rows */
                else if (use_quiet && (j % 100 == 0))
@@ -5018,15 +5019,25 @@ initPopulateTable(PGconn *con, const char *table, int64 
base,
                        /* have we reached the next interval (or end)? */
                        if ((j == total) || (elapsed_sec >= log_interval * 
LOG_STEP_SECONDS))
                        {
-                               chars = fprintf(stderr, INT64_FORMAT " of " 
INT64_FORMAT " tuples (%d%%) of %s done (elapsed %.2f s, remaining %.2f s)%c",
+                               chars = fprintf(stderr, INT64_FORMAT " of " 
INT64_FORMAT " tuples (%d%%) of %s done (elapsed %.2f s, remaining %.2f s)",
                                                                j, total,
                                                                (int) ((j * 
100) / total),
-                                                               table, 
elapsed_sec, remaining_sec, eol);
+                                                               table, 
elapsed_sec, remaining_sec);
 
                                /* skip to the next interval */
                                log_interval = (int) ceil(elapsed_sec / 
LOG_STEP_SECONDS);
                        }
                }
+
+               /*
+                * If the previous progress message is longer than the current 
one,
+                * add spaces to the current line to fully overwrite any 
remaining
+                * characters from the previous message.
+                */
+               if (prev_chars > chars)
+                       fprintf(stderr, "%*c", prev_chars - chars, ' ');
+               fputc(eol, stderr);
+               prev_chars = chars;
        }
 
        if (chars != 0 && eol != '\n')
-- 
2.47.0

Reply via email to