Re: libpq debug log

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: "iwata(dot)aya(at)fujitsu(dot)com" <iwata(dot)aya(at)fujitsu(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "tgl(at)sss(dot)pgh(dot)pa(dot)us" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>, "pchampion(at)pivotal(dot)io" <pchampion(at)pivotal(dot)io>, "jdoty(at)pivotal(dot)io" <jdoty(at)pivotal(dot)io>, "raam(dot)soft(at)gmail(dot)com" <raam(dot)soft(at)gmail(dot)com>, "nagaura(dot)ryohei(at)fujitsu(dot)com" <nagaura(dot)ryohei(at)fujitsu(dot)com>, "nagata(at)sraoss(dot)co(dot)jp" <nagata(at)sraoss(dot)co(dot)jp>, "peter(dot)eisentraut(at)2ndquadrant(dot)com" <peter(dot)eisentraut(at)2ndquadrant(dot)com>, 'Kyotaro HORIGUCHI' <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>
Subject: Re: libpq debug log
Date: 2020-10-26 16:23:13
Message-ID: 20201026162313.GA22502@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

Attachment Content-Type Size
v9-0001-libpq-trace.patch text/x-diff 22.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Dilger 2020-10-26 16:28:45 Re: new heapcheck contrib module
Previous Message Konstantin Knizhnik 2020-10-26 16:20:46 Re: libpq compression