Re: pgbench - use pg logging capabilities

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench - use pg logging capabilities
Date: 2020-01-07 09:32:41
Message-ID: alpine.DEB.2.21.2001070937350.31411@pseudo
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Bonjour Michaël,

>>> I think that I would just remove the "debug" variable defined in
>>> pgbench.c all together, and switch the messages for the duration and the
>>> one in executeMetaCommand to use info-level logging..
>>
>> Ok, done.
>
> Thanks. The output then gets kind of inconsistent when using --debug:
> pgbench: client 2 executing script "<builtin: TPC-B (sort of)>"
> client 2 executing \set aid
> client 2 executing \set bid
> client 2 executing \set tid
> client 2 executing \set delta
>
> My point was to just modify the code so as this uses pg_log_debug(),
> with a routine doing some reverse-engineering of the Command data to
> generate a string to show up in the logs. Sorry for the confusion..
> And there is no need to use __pg_log_level either which should remain
> internal to logging.h IMO.

For the first case with the output you point out, there is a loop to
generate the output. I do not think that we want to pay the cost of
generating the string and then throw it away afterwards when not under
debug, esp as string manipulation is not that cheap, so we need to enter
the thing only when under debug. However, there is no easy way to do that
without accessing __pg_log_level. It could be hidden in a macro to create,
but that's it.

For the second case I called pg_log_debug just once.

> We'd likely want a similar business in syntax_error() to be fully
> consistent with all other code paths dealing with an error showing up
> before exiting.

The syntax error is kind of complicated because there is the location
information which is better left as is, IMHO. I moved remainder to a
PQExpBuffer and pg_log_fatal.

> No idea what others think here. I may be too much pedantic.

Maybe a little:-)

Note that I submitted another patch to use PQExpBuffer wherever possible
in pgbench, especially to get rid of doubtful snprintf/strlen patterns.

Patch v5 attached tries to follow your above suggestions.

--
Fabien.

Attachment Content-Type Size
pgbench-logging-5.patch text/x-diff 41.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2020-01-07 10:32:29 Re: WIP: System Versioned Temporal Table
Previous Message Michael Paquier 2020-01-07 08:24:26 Re: pgbench - use pg logging capabilities