RE: libpq debug log

Hi Alvaro san,

Thank you for your update :)

> Opinions? I experimented by patching psql as below (not intended for
> commit) and it looks good. Only ErrorResponse prints the terminator as a
> control character, or something:
I check code, changes and email. I agree with all of this.
I will review code, feature and performance if it is needed more closely.
(I'll start it next week.)

Regards,
Aya Iwata

> -----Original Message-----
> From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
> Sent: Thursday, September 17, 2020 5:42 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 like this patch, and I think we should have it. I have updated it, as it had
> conflicts.
>
> In 0002, I remove use of ftime(), because that function is obsolete.
> However, with that change we lose printing of milliseconds in the trace lines.
> I think that's not great, but I also think we can live without them until
> somebody gets motivated to write the code for that. It seems a little messy
> so I'd prefer to leave that for a subsequent commit.
> (Maybe we can just use pg_strftime.)
>
> Looking at the message type tables, I decided to get rid of the "bf"
> table, which had only two bytes, and instead put CopyData and CopyDone in
> the other two tables. That seems simpler. Also, the COMMAND_x_MAX
> macros were a bit pointless; I just expanded at the only caller of each, using
> lengthof(). And since the message type is unsigned, there's no need to do "c
> >= 0" since it must always be true.
>
> I added setlinebuf() to the debug file. Works better than without, because
> "tail -f /tmp/libpqtrace.log" works as you'd expect.
>
> One thing that it might be good to do is to avoid creating the message type
> tables as const but instead initialize them if and only if tracing is enabled, on
> the first call. I think that would not only save space in the constant area of
> the library for the 99.99% of the cases where tracing isn't used, but also make
> the initialization code be more sensible (since presumably you wouldn't have
> to initialize all the
> zeroes.)
>
> Opinions? I experimented by patching psql as below (not intended for
> commit) and it looks good. Only ErrorResponse prints the terminator as a
> control character, or something:
>
> 2020-09-16 13:27:29.072 -03 < ErrorResponse 117 S "ERROR" V "ERROR" C
> "42704" M "no existe el slot de replicación «foobar»" F "slot.c" L "408" R
> "ReplicationSlotAcquireInternal" ^@
>
>
> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index
> 8232a0143b..01728ba8e8 100644
> --- a/src/bin/psql/startup.c
> +++ b/src/bin/psql/startup.c
> @@ -301,6 +301,11 @@ main(int argc, char *argv[])
>
> psql_setup_cancel_handler();
>
> + {
> + FILE *trace = fopen("/tmp/libpqtrace.log", "a+");
> + PQtrace(pset.db, trace);
> + }
> +
> PQsetNoticeProcessor(pset.db, NoticeProcessor, NULL);
>
> SyncVariables();
>
> --
> Álvaro Herrera https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services