From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Tatsuo Ishii <ishii@postgresql.org>
Cc: btogiwarayuushi@oss.nttdata.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: Fix for Extra Parenthesis in pgbench progress message
Date: Sat, 9 Nov 2024 14:46:00 +0900
Message-ID: <95f1100e-e59b-41b7-a406-f865c2e69db7@oss.nttdata.com> (raw)
In-Reply-To: <20241108.114713.484828673471024549.ishii@postgresql.org>
References: <0f7ac0fa-f26b-49dc-b9a0-d35c29ace3a5@oss.nttdata.com>
<20241107.104143.1498084100859794666.ishii@postgresql.org>
<85fe426d-248a-47f7-8784-22af811ee572@oss.nttdata.com>
<20241108.114713.484828673471024549.ishii@postgresql.org>
On 2024/11/08 11:47, Tatsuo Ishii wrote:
> I think you need to adjust
>
> fprintf(stderr, "%*c\r", chars - 1, ' '); /* Clear the current line */
>
> to:
>
> fprintf(stderr, "%*c\r", chars, ' '); /* Clear the current line */
>
> since now chars does not consider the EOL char. By clearing chars - 1,
> the closing parenthesis will be left on the screen.
You're right! I've updated the patch and attached v4.
About the back-patch: initially, there were concerns it might be too invasive,
as you told upthread. However, after our discussion, I think the latest version
is straightforward enough, and I'm okay with back-patching it. Thoughts?
Since the minor release freeze is already in effect, if we commit this patch,
we'll need to wait until the next minor releases are out next week before
the commit and back-patch.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:v4-0001-pgbench-Ensure-previous-progress-message-is-fully.patch (text/plain, 3.2K) download
From b55547a5db704a24dd6d2f8d01918f420626705c Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Fri, 8 Nov 2024 00:34:55 +0900
Subject: [PATCH v4] 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/9a9b8b95b6a709877ae48ad5b0c59bb9@oss.nttdata.com
---
src/bin/pgbench/pgbench.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.cindex ba247f3f85..85e7b68baa 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,19 +5019,29 @@ 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')
- fprintf(stderr, "%*c\r", chars - 1, ' '); /* Clear the current line */+ fprintf(stderr, "%*c\r", chars, ' '); /* Clear the current line */
if (PQputline(con, "\\.\n"))
pg_fatal("very last PQputline failed");
--
2.47.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, ishii@postgresql.org, btogiwarayuushi@oss.nttdata.com
Subject: Re: Fix for Extra Parenthesis in pgbench progress message
In-Reply-To: <95f1100e-e59b-41b7-a406-f865c2e69db7@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox