Re: libpq debug log

At Tue, 9 Feb 2021 08:10:19 +0000, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com> wrote in 
> From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
> > > (45)
> > This looks like a fusion of PQtrace and PQtraceEX. By the way, the
> > timestamp flag is needed at log emittion. So we can change the state
> > anytime.
> > 
> > PQtrace(conn, of);
> > PQtraceSetFlags(conn, PQTRACE_SUPPRESS_TIMESTAMPS);
> > <logging without timestamps>
> > PQtraceSetFlags(conn, 0);
> > <logging with timestamps>
> 
> I find this better because the application does not have to call PQuntrace() and PQtrace() again to enable/disable
timestampoutput, which requires passing the FILE pointer again.  (I don't imagine applications would repeatedly turn
loggingon and off in practice, though.)
 

Thanks and Yes, I also don't imagine that users change the timestmp
state repeatedly:p It's just a example.

> > > (46)
> > >
> > > If skipLogging is intended for use with backend -> frontend messages only,
> > shouldn't it be placed in conn->b_msg?
> > 
> > The name skipLogging is somewhat obscure. The flag doesn't inhibit all
> > logs from being emitted.  It seems like it represents how far bytes
> > the logging mechanism consumed for the limited cases. Thus, I think it
> > can be a cursor variable like inCursor.
> > 
> > If we have conn->be_msg->inLogged, for example, pqGetc and
> > pqLogMessageByte1() are written as the follows.
> > 
> > pqGetc(char *result, PGconn *conn)
> > {
> >     if (conn->inCursor >= conn->inEnd)
> >         return EOF;
> > 
> >     *result = conn->inBuffer[conn->inCursor++];
> > 
> >     if (conn->Pfdebug)
> >         pqLogMessageByte1(conn, *result);
> > 
> >     return 0;
> > }
> > 
> > pqLogMessageByte1(...)
> > {
> >    switch()
> >    {
> >      case LOG_FIRST_BYTE:
> >        /* No point in logging already logged bytes */
> >        if (conn->be_msg->inLogged >= conn->inCursor)
> >          return;
> >    ...
> >    }
> >    conn->be_msg->inLogged = conn->inCursor;
> > }
> 
> This looks better design because stuff like skipLogging is an internal state of logging facility whose use should be
restrictedto fe-logging.c.
 
> 
> > (pqCheckInBufferSpace needs to adjust inLogged.)
> > 
> > I'm not sure this is easier to read than the skipLogging.
> 
> I'd like Iwata-san to evaluate this and decide whether to take this approach or the current one.

+1

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center