Re: libpq debug log

From: "'alvherre(at)alvh(dot)no-ip(dot)org'" <alvherre(at)alvh(dot)no-ip(dot)org>
To: "iwata(dot)aya(at)fujitsu(dot)com" <iwata(dot)aya(at)fujitsu(dot)com>
Cc: "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com>, 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq debug log
Date: 2021-03-10 13:31:27
Message-ID: 20210310133127.GA18076@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-Mar-10, iwata(dot)aya(at)fujitsu(dot)com wrote:

> Hi all,
>
> Following all reviewer's advice, I have created a new patch.
>
> In this patch, I add only two tracing entry points; I call pqTraceOutputMsg(PGconn conn, int msgCursor, PGCommSource commsource) in pqParseInput3 () and pqPutMsgEnd () to output log.
> The argument contains message first byte offset called msgCursor because it is simpler to specify the buffer pointer in the caller.
>
> In pqTraceOutputMsg(), the content common to all protocol messages (first timestamp, < or >, first 1 byte, and length) are output first and after that each protocol message contents is output. I referred to pqParseInput3 () , fe-exec.c and documentation page for that part.
>
> This fix almost eliminates if(conn->Pfdebug) that was built into the code for other features.

This certainly looks much better! Thanks for getting it done so
quickly.

I didn't review the code super closely. I do see a compiler warning:

/pgsql/source/pipeline/src/interfaces/libpq/libpq-trace.c: In function 'pqTraceOutputNchar':
/pgsql/source/pipeline/src/interfaces/libpq/libpq-trace.c:373:2: warning: argument 1 null where non-null expected [-Wnonnull]
memcpy(v, buf + *cursor, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and then when I try to run the "libpq_pipeline" program from the other
thread, I get a crash with this backtrace:

Program received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:288
288 ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No existe el fichero o el directorio.
(gdb) bt
#0 __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:288
#1 0x00007ffff7f9b50b in pqTraceOutputNchar (buf=buf(at)entry=0x555555564660 "P",
LogEnd=LogEnd(at)entry=42, cursor=cursor(at)entry=0x7fffffffdeb4, pfdebug=0x555555569320, len=1)
at /pgsql/source/pipeline/src/interfaces/libpq/libpq-trace.c:373
#2 0x00007ffff7f9bc25 in pqTraceOutputMsg (conn=conn(at)entry=0x555555560260, msgCursor=<optimized out>,
commsource=commsource(at)entry=MSGDIR_FROM_FRONTEND)
at /pgsql/source/pipeline/src/interfaces/libpq/libpq-trace.c:476
#3 0x00007ffff7f96280 in pqPutMsgEnd (conn=conn(at)entry=0x555555560260)
at /pgsql/source/pipeline/src/interfaces/libpq/fe-misc.c:533
...

The attached patch fixes it, though I'm not sure that that's the final
form we want it to have (since we'd alloc and free repeatedly, making it
slow -- perhaps better to use a static, or ...? ).

--
Álvaro Herrera Valdivia, Chile
Essentially, you're proposing Kevlar shoes as a solution for the problem
that you want to walk around carrying a loaded gun aimed at your foot.
(Tom Lane)

Attachment Content-Type Size
fixsegv.patch text/x-diff 815 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2021-03-10 13:47:35 Re: get rid of <foreignphrase> tags in the docs?
Previous Message David Steele 2021-03-10 13:24:50 Re: error_severity of brin work item