Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

On 10-08-2018 17:19, Arthur Zakirov wrote:
> On Fri, Aug 10, 2018 at 04:46:04PM +0300, Marina Polyakova wrote:
>> > +1 from me to keep initial name "pgbench_error". "pgbench_log" for new
>> > function looks nice to me. I think it is better than just "log",
>> > because "log" may conflict with natural logarithmic function (see "man 3
>> > log").
>> 
>> Do you think that pgbench_log (or another whose name speaks only about
>> logging) will look good, for example, with FATAL? Because this means 
>> that
>> the logging function also processes errors and calls exit(1) if 
>> necessary..
> 
> Yes, why not. "_log" just means that you want to log some message with
> the specified log level. Moreover those messages sometimes aren't 
> error:
> 
> pgbench_error(LOG, "starting vacuum...");

"pgbench_log" is already used as the default filename prefix for 
transaction logging.

>> > I agree with Fabien. Calling pgbench_error() inside pgbench_error()
>> > could be dangerous. I think "fmt" checking could be removed, or we may
>> > use Assert()
>> 
>> I would like not to use Assert in this case because IIUC they are 
>> mostly
>> used for testing.
> 
> I'd vote to remove this check at all. I don't see any place where it is
> possible to call pgbench_error() passing empty "fmt".

pgbench_error(..., "%s", PQerrorMessage(con)); ?

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company