RE: libpq debug log

Hi Alvaro san,

Thank you for your email.
I will review this updated patch and I will let you know when I complete.
Please wait a moment.

Best regards,
Aya Iwata

> -----Original Message-----
> From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
> Sent: Tuesday, October 27, 2020 1:23 AM
> To: Iwata, Aya/岩田 彩 <iwata(dot)aya(at)fujitsu(dot)com>
> Cc: pgsql-hackers(at)postgresql(dot)org; tgl(at)sss(dot)pgh(dot)pa(dot)us;
> robertmhaas(at)gmail(dot)com; pchampion(at)pivotal(dot)io; jdoty(at)pivotal(dot)io;
> raam(dot)soft(at)gmail(dot)com; Nagaura, Ryohei/永浦 良平
> <nagaura(dot)ryohei(at)fujitsu(dot)com>; nagata(at)sraoss(dot)co(dot)jp;
> peter(dot)eisentraut(at)2ndquadrant(dot)com; 'Kyotaro HORIGUCHI'
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>; Jamison, Kirk/ジャミソン カーク
> <k(dot)jamison(at)fujitsu(dot)com>
> Subject: Re: libpq debug log
>
> Hello Aya Iwata
>
> I've been hacking at this patch again. There were a few things I wasn't too
> clear about, so I reordered the code and renamed the routines to try to make it
> easier to follow.
>
> One thing I didn't like very much is that all the structures and enums were
> exposed to the world in libq-int.h. Because some enum members have
> pretty generic names, I didn't like that much, so I moved the whole thing to
> fe-misc.c, and renamed the structs. Also, the arrays don't take space unless
> PQtrace() is called. (This is not what I was talking about in my previous
> message. The idea I was trying to explain in my previous message cannot
> possibly work, so I abandoned it.)
>
> I also renamed functions to make it clear which handles frontend and which
> handles backend. With that, it was pretty obvious that we had an "reset BE
> message" in the routine to handle FE, and some clearing of FE in the code that
> handles BE. I fixed things in a way that I think makes the most sense.
>
> I noticed that it's theoretically possible to have a FE message so large (more
> than MAXPGPATH pieces) that it would overrun the array; so I added a "print
> message" call after adding one piece, to avoid this. Also, why MAXPGPATH?
> I added a new symbol MAX_FRONTEND_MSGS for this purpose.
>
> There are some things still to do:
>
> 0. I added a XXX comment to pqFlush. Because we're storing messages in
> fe_msgs that point to the libpq buffer, is it possible to end up with trace
> messages that are pointing into outBuffer bytes that are already sent, and
> perhaps even overwritten with newer bytes? Maybe not, but it's unclear.
> Should we do pqLogFrontendMsg() preventively to avoid this problem?
>
> 1. Is the handling of protocol 2 correct? Since it's completely separate from
> protocol 3, I have not even looked at what it produces.
> But the fact that pqLogMsgByte1 completely ignores the "commsource"
> argument makes me suspect that it's not correct.
> 1a. How do we even test protocol 2 handling?
>
> 2. We need a mode to suppress print of time; this would be useful to be able to
> test libpq at a low level. Maybe instead of time we can print a
> monotonically-increasing packet sequence number. With this, we could
> easily add tests for libpq itself. What user interface do we want for this?
> Maybe we can add an "bits32 flags" parameter to PQtrace(), with one bit for
> this use.
>
> 3. COPY ... (FORMAT BINARY) emits "invalid protocol" ... not good.
>
> 4. Even in text format, COPY output is not very useful. How can we improve
> that?
>
> 5. Error messages are still printing the terminating zero byte. I suggest that
> it should be suppressed.
>
> 6. Let's redefine pqTraceMaybeBreakLine() (I renamed from
> pqLogLineBreak): If message is complete, print a newline; if message is not
> complete, print a " ". Then, remove the whitespace after printing each
> element. This should lead to lines that don't have an useless " "
> at the end.
>
> 7. Why does it make sense to call pqTraceMaybeBreakLine when
> commsource=FROM_FRONTEND?