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

On Thu, Aug 09, 2018 at 06:17:22PM +0300, Marina Polyakova wrote:
> > * ErrorLevel
> >
> > If ErrorLevel is used for things which are not errors, its name should
> > not include "Error"? Maybe "LogLevel"?
>
> On the one hand, this sounds better for me too. On the other hand, will not
> this be in some kind of conflict with error level codes in elog.h?..

I think it shouldn't because those error levels are backends levels.
pgbench is a client side utility with its own code, it shares some code
with libpq and other utilities, but elog.h isn't one of them.

> > This point also suggest that maybe "pgbench_error" is misnamed as well
> > (ok, I know I suggested it in place of ereport, but e stands for error
> > there), as it is called on errors, but is also on other things. Maybe
> > "pgbench_log"? Or just simply "log" or "report", as it is really an
> > local function, which does not need a prefix? That would mean that
> > "pgbench_simple_error", which is indeed called on errors, could keep
> > its initial name "pgbench_error", and be called on errors.
>
> About the name "log" - we already have the function doLog, so perhaps the
> name "report" will be better.. But like with ErrorLevel will not this be in
> some kind of conflict with ereport which is also used for the levels
> DEBUG... / LOG / INFO?

+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").

> > pgbench_error calls pgbench_error. Hmmm, why not.

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() or fprintf()+exit(1) at least.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company