On 13-06-2018 22:44, Fabien COELHO wrote:
Hello Marina,
I suppose that this is related; because of my patch there may be a lot
of such code (see v7 in [1]):
- fprintf(stderr,
- "malformed variable \"%s\" value:
\"%s\"\n",
- var->name, var->svalue);
+ if (debug_level >= DEBUG_FAILS)
+ {
+ fprintf(stderr,
+ "malformed variable \"%s\" value:
\"%s\"\n",
+ var->name, var->svalue);
+ }
- if (debug)
+ if (debug_level >= DEBUG_ALL)
fprintf(stderr, "client %d sending %s\n", st->id, sql);
I'm not sure that debug messages needs to be kept after debug, if it
is about debugging pgbench itself. That is debatable.
AFAICS it is not about debugging pgbench itself, but about more detailed
information that can be used to understand what exactly happened during
its launch. In the case of errors this helps to distinguish between
failures or errors by type (including which limit for retries was
violated and how far it was exceeded for the serialization/deadlock
errors).
The code adapts/duplicates existing server-side "ereport" stuff and
brings it to the frontend, where the logging needs are somehow quite
different.
I'd prefer to avoid duplication and/or have some code sharing.
I was recommended to use the same interface in [3]:
On elog/errstart: we already have a convention for what ereport()
calls look like; I suggest to use that instead of inventing your
own.
The "elog" interface already exists, it is not an invention. "ereport"
is a hack which is somehow necessary in some cases. I prefer a simple
function call if possible for the purpose, and ISTM that this is the
case.
That is a lot of complication which are justified server side
where logging requirements are special, but in this case I see it as
overkill.
I think we need ereport() if we want to make detailed error messages
(see examples in [1])..
If it really needs to be duplicated, I'd suggest to put all this
stuff in separated files. If we want to do that, I think that it
would belong to fe_utils, and where it could/should be used by all
front-end programs.
I'll try to do it..
Dunno. If you only need one "elog" function which prints a message to
stderr and decides whether to abort/exit/whatevrer, maybe it can just
be kept in pgbench. If there are are several complicated functions and
macros, better with a file. So I'd say it depends.
So my current view is that if you only need an "elog" function, it is
simpler to add it to "pgbench.c".
Thank you!
For logging purposes, ISTM that the "elog" macro interface is nicer,
closer to the existing "fprintf(stderr", as it would not introduce
the
additional parentheses hack for "rest".
I was also recommended to use ereport() instead of elog() in [3]:
Probably. Are you hoping that advises from different reviewers should
be consistent? That seems optimistic:-)
To make the patch committable there should be no objection to it..
[1]
https://www.postgresql.org/message-id/c89fcc380a19380260b5ea463efc1416%40postgrespro.ru
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
