Re: pgbench - use pg logging capabilities

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
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-06 08:15:54
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Jan 03, 2020 at 01:01:18PM +0100, Fabien COELHO wrote:
> 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.

The variable names in Command are confusing IMO...

> Ok. I homogeneised another similar message.
> Patch v3 attached hopefully fixes all of the above.

+ pg_log_error("gaussian parameter must be at least "
+ "%f (not %f)", MIN_GAUSSIAN_PARAM, param);
I would keep all the error message strings to be on the same line.
That makes grepping for them easier on the same, and that's the usual
convention even if these are larger than 72-80 characters.

#ifdef DEBUG
- printf("shell parameter name: \"%s\", value: \"%s\"\n", argv[1], res);
+ pg_log_debug("%s: shell parameter name: \"%s\", value: \"%s\"", argv[0], argv[1], res);
Worth removing this ifdef?

- fprintf(stderr, "%s", PQerrorMessage(con));
+ pg_log_fatal("unexpected copy in result");
+ pg_log_error("%s", PQerrorMessage(con));
- fprintf(stderr, "%s", PQerrorMessage(con));
+ pg_log_fatal("cannot count number of branches");
+ pg_log_error("%s", PQerrorMessage(con));
These are inconsistent with the rest, why not combining both?

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..

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-01-06 08:20:53 Re: pg_basebackup fails on databases with high OIDs
Previous Message Peter Eisentraut 2020-01-06 08:07:26 pg_basebackup fails on databases with high OIDs