RE: libpq debug log - Mailing list pgsql-hackers

Hi Horiguchi-san,

Thank you for your reply and suggestions.

> > 1. Expand PQtrace() facility and improve libpq logging.
> >
> > 2. Prepare "output level". There are 3 type of levels;
> >     - TRADITIONAL    :  if set, outputs protocol messages
> >     - LEVEL1        :  if set, outputs phase and time
> >     - LEVEL2        :  if set, outputs both info TRADITIONAL and LEVEL1
> 
> You appear to want to segregate the "traditional" output from what you want
> to emit. At least Tom is explicitly suggesting to throw away the hypothtical
> use cases for it. You may sort out what kind of information you/we want to
> emit as log messages from scratch:p
> 
> You may organize log kinds into hierachical levels like server log messages
> or into orthogonal types that are individually turned on. But it is not
> necessarily be a parameter of a logging processor. (mentioned below)

It is intended for old application users who use PQtrace() expect the existing/default/traditional log output style.
That’s why I separated other information(ex. phase and timestamp).
But since you mentioned the level is not necessary, 
I will follow your advice and include those information in the reformatted PQtrace().


> > 3. Add "output phase" information; This is the processing flow. (ex.
> > When PQexec() start and end )
> 
> What is the advantage of it against just two independent messages like
> PQexec_start and PQexec_end? (I don't see any advantage.)

I think the purpose of this logging improvement is to make it useful for analysis at performance deterioration.
When query delay happens, we want to know from which side(server or client) is the cause of it, 
and then people want to know which process takes time.
I think the phase and time information are useful for diagnosis.
For example, when command processing function (ex. PQexec()) etc. start/end 
and when client receive/send protocol messages.
/*my intended output here */


> > 4. Change output method to callback style; Default is stdout, and prepare
> other callback functions that will be used more frequently.
> 
> Are you going to make something less-used the default behavior? I think no
> one is opposing rich functionality as far as it is replaceable.
I am sorry, my explanation was not clear.
I just wanted to say I intend to provide several output method functions which users likely need.
ex. output to stdout, or output to file, or output to log directory.

 
> > 5. Initialization method;
> > In current one: PQtrace(PGconn *conn, FILE *stream); Proposed change:
> > PQtraceEx(PGconn *conn, FILE *stream, PQloggingProcessor callback_func
> > , void *callback_arg, PGLogminlevel level);
> 
> I'm not sure we should add a new *EX() function. Couldn't we just change the
> signature of PQtrace()?
I intended to add new *EX() function for compatibility purposes specially for old version of libpq. 
I would like to avoid making changes to old applications for this.
But if we insist on changing the PQtrace() itself, then I will follow your advice.

> 
> callback_funs seems to be a single function. I think it's better to have
> individual function for each message type.  Not
> callback_func(PQLOG_EXEC_START, param_1, param_2,...)  ,but
> PQloggingProcessor.PQexec_start(param_1, param_2, ...).
> 
> It is because the caller can simply pass values in its own type to the function
> without casting or other transformations and their types are checked
> statically.
> 
> I also think it's better that logger-specific paramters are not passed in
> this level.  Maybe stream and level are logger-specific paratmer, which can
> be combined into callback_arg, or can be given via an API function.

Thank you for your advice. I will consider it.

Regards,
Aya Iwata