Re: libpq debug log
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: "iwata.aya@fujitsu.com" <iwata.aya@fujitsu.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "pchampion@pivotal.io" <pchampion@pivotal.io>, "jdoty@pivotal.io" <jdoty@pivotal.io>, "raam.soft@gmail.com" <raam.soft@gmail.com>, "nagaura.ryohei@fujitsu.com" <nagaura.ryohei@fujitsu.com>, "nagata@sraoss.co.jp" <nagata@sraoss.co.jp>, "peter.eisentraut@2ndquadrant.com" <peter.eisentraut@2ndquadrant.com>, 'Kyotaro HORIGUCHI' <horiguchi.kyotaro@lab.ntt.co.jp>, "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Date: 2020-10-26T16:23:13Z
Lists: pgsql-hackers
Attachments
- v9-0001-libpq-trace.patch (text/x-diff)
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?
Commits
-
Rename PQtraceSetFlags() to PQsetTraceFlags().
- d0e750c0acaf 14.0 landed
-
Suppress length of Notice/Error msgs in PQtrace regress mode
- e7e341409a3d 14.0 landed
-
Strip file names reported in error messages on Windows, too.
- 53aafdb9ff6a 14.0 landed
-
Fix setvbuf()-induced crash in libpq_pipeline
- a68a894f0198 14.0 landed
-
libpq_pipeline: Must strdup(optarg) to avoid crash
- dde1a35aee62 14.0 landed
-
Remove setvbuf() call from PQtrace()
- 6ec578e60101 14.0 landed
-
Initialize conn->Pfdebug to NULL when creating a connection
- aba24b51cc1b 14.0 landed
-
Disable force_parallel_mode in libpq_pipeline
- a6d3dea8e5e0 14.0 landed
-
libpq_pipeline: add PQtrace() support and tests
- 7bebd0d00998 14.0 landed
-
Improve PQtrace() output format
- 198b3716dba6 14.0 landed
-
Re-simplify management of inStart in pqParseInput3's subroutines.
- d3a557894ce0 11.12 landed
- d2be6cdc55ad 10.17 landed
- a98e53e10dd5 9.6.22 landed
- 56defbdd0f4e 12.7 landed
- 51c54bb60309 14.0 landed
- 3580b4a0cde0 13.3 landed