From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgbench - use pg logging capabilities |
Date: | 2020-01-03 12:01:18 |
Message-ID: | alpine.DEB.2.21.2001031213190.3482@pseudo |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello Peter,
>> Compared to v1 I have also made a few changes to be more consistent when
>> using fatal/error/info.
>
> The renaming of debug to debug_level seems unnecessary and unrelated.
Indeed. It was when I used "debug" as a shorthand for "pg_log_debug".
> In runShellCommand(), you removed some but not all argv[0] from the output
> messages. I'm not sure what the intent was there.
Without looking at the context I thought that argv[0] was the program
name, which is not the case here. I put it back everywhere, including the
DEBUG message.
> I would also recommend these changes:
>
> - pg_log_fatal("query failed: %s", sql);
> - pg_log_error("%s", PQerrorMessage(con));
> + pg_log_fatal("query failed: %s", PQerrorMessage(con));
> + pg_log_info("query was: %s", sql);
>
> This puts the most important information first.
Ok.
> - pg_log_error("connection to database \"%s\" failed", dbName);
> - pg_log_error("%s", PQerrorMessage(conn));
> + pg_log_error("connection to database \"%s\" failed: %s",
> + dbName, PQerrorMessage(conn));
>
> Line break here is unnecessary.
Ok. I homogeneised another similar message.
Patch v3 attached hopefully fixes all of the above.
--
Fabien.
Attachment | Content-Type | Size |
---|---|---|
pgbench-logging-3.patch | text/x-diff | 38.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2020-01-03 12:07:33 | Re: pgbench - allow to create partitioned tables |
Previous Message | Amit Kapila | 2020-01-03 12:01:03 | sidewinder has one failure |