Re: pgbench - use pg logging capabilities - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: pgbench - use pg logging capabilities
Date
Msg-id alpine.DEB.2.21.2001070937350.31411@pseudo
Whole thread Raw
In response to Re: pgbench - use pg logging capabilities  (Michael Paquier <michael@paquier.xyz>)
Responses Re: pgbench - use pg logging capabilities  (Fabien COELHO <coelho@cri.ensmp.fr>)
List 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

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: pgbench - use pg logging capabilities
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: WIP: System Versioned Temporal Table