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

Hello Alvaro,

> For context: in the backend, elog() is only used for internal messages
> (i.e. "can't-happen" conditions), and ereport() is used for user-facing
> messages. There are many things ereport() has that elog() doesn't, such
> as additional message fields (HINT, DETAIL, etc) that I think could have
> some use in pgbench as well. If you use elog() then you can't have that.
> [...]

Ok. Then forget elog, but I'm pretty against having a kind of ereport
which looks greatly overkill to me, because:

(1) the syntax is pretty heavy, and does not look like a function.

(2) the implementation allocates a string buffer for the message
this is greatly overkill for pgbench which only needs to print
to stderr once.

This makes sense server-side because the generated message may be output
several times (eg stderr, file logging, to the client), and the
implementation has to work with cpp implementations which do not handle
varags (and maybe other reasons).

So I would be in favor of having just a simpler error function.
Incidentally, one already exists "pgbench_error" and could be improved,
extended, replaced. There is also "syntax_error".

> One thing that just came to mind is that pgbench uses some src/fe_utils
> stuff. I hope having ereport() doesn't cause a conflict with that ...

Currently ereport does not exists client-side. I do not think that this
patch is the right moment to decide to do that. Also, there are some
"elog" in libpq, but they are out with a "#ifndef FRONTEND".

> BTW I think abort() is not the right thing, as it'll cause core dumps if
> enabled. Why not just exit(1)?

Yes, I agree and already reported that.

Conclusion:

My current opinion is that I'm pretty against bringing "ereport" to the
front-end on this specific pgbench patch. I agree with you that "elog"
would be misleading there as well, for the arguments you developed above.

I'd suggest to have just one clean and simple pgbench internal function to
handle errors and possibly exit, debug... Something like

void pgb_error(FATAL, "error %d raised", 12);

Implemented as

void pgb_error(int/enum XXX level, const char * format, ...)
{
test level and maybe return immediately (eg debug);
print to stderr;
exit/abort/return depending;
}

Then if some advanced error handling is introduced for front-end programs,
possibly through some macros, then it would be time to improve upon that.

--
Fabien.