RE: libpq debug log - Mailing list pgsql-hackers
From | iwata.aya@fujitsu.com |
---|---|
Subject | RE: libpq debug log |
Date | |
Msg-id | TY2PR01MB196349912409C4491F2B6943EAE10@TY2PR01MB1963.jpnprd01.prod.outlook.com Whole thread Raw |
In response to | Re: libpq debug log (Alvaro Herrera <alvherre@alvh.no-ip.org>) |
List | pgsql-hackers |
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@alvh.no-ip.org> > Sent: Tuesday, October 27, 2020 1:23 AM > To: Iwata, Aya/岩田 彩 <iwata.aya@fujitsu.com> > Cc: pgsql-hackers@postgresql.org; tgl@sss.pgh.pa.us; > robertmhaas@gmail.com; pchampion@pivotal.io; jdoty@pivotal.io; > raam.soft@gmail.com; Nagaura, Ryohei/永浦 良平 > <nagaura.ryohei@fujitsu.com>; nagata@sraoss.co.jp; > peter.eisentraut@2ndquadrant.com; 'Kyotaro HORIGUCHI' > <horiguchi.kyotaro@lab.ntt.co.jp>; Jamison, Kirk/ジャミソン カーク > <k.jamison@fujitsu.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?
pgsql-hackers by date: