Horton

Re: libpq debug log

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: "Iwata, Aya" <iwata.aya@jp.fujitsu.com>
Cc: "Jamison, Kirk" <k.jamison@jp.fujitsu.com>, "Nagaura, Ryohei" <nagaura.ryohei@jp.fujitsu.com>, Jacob Champion <pchampion@pivotal.io>, Jim Doty <jdoty@pivotal.io>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>, "nagata@sraoss.co.jp" <nagata@sraoss.co.jp>, Tom Lane <tgl@sss.pgh.pa.us>, Haribabu Kommi <kommi.haribabu@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Date: 2019-02-22T15:41:02Z
Lists: pgsql-hackers 
On Thu, Feb 21, 2019 at 7:52 PM Iwata, Aya <iwata.aya@jp.fujitsu.com> wrote:
> > I'm not really sure that I like the design of this patch in any way.
> Aside from problems with my current documentation which I will fix,
> could you explain more detail about the problem of the design?
> I would like to improve my current implementation based from feedback.

Well, I believe that what you've got here is something that could,
perhaps, be occasionally useful.  However, I don't think it would be
useful to very many people very often, and we'd still have to maintain
the code, so that's not a great situation.

We already have a PQtrace() facility that could be improved, and it
has been previously suggested that you work on improving this facility
rather than inventing something new.  I still think that's a good
idea.  Instead you've created a second way of producing similar
information, and then coupled it to very specific ideas about how that
information should be logged: it is triggered by new libpq parameters,
there is log rotation logic, etc.  Those might not be right for
everyone, and there's no flexibility in the mechanism.

I am not sure that it's a good idea to have facilities that write to
the local filesystem that can be triggered by libpq parameters.  Seems
like that might have possible security consequences, or at least annoy
people who want to accept connection strings from users without having
to sanitize them for these sorts of options.

I do sometimes want to know  what's going on at the protocol level.
Sometimes it's possible to use wireshark for that (as mentioned
upthread) and when it isn't, the thing I'd really like is for the
command-line clients that already exist have an option to enable
PQtrace without me having to hack the C code.  We could go through and
add a long-form command-line option, --libpq-trace, to every
command-line tool we ship, for example.  Then we could, as a separate
patch, improve the format of that tracing output.  For me that would
be more useful than this.

I don't necessarily think there's anything deeply wrong with this
approach.  It's not like your patch will bring about the end of
civilization or anything like that.  It just doesn't excite me very
much.  And since I agree that we have a problem in this area, I would
ideally like to have a solution to that problem that I feel excited
about.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Rename PQtraceSetFlags() to PQsetTraceFlags().

  2. Suppress length of Notice/Error msgs in PQtrace regress mode

  3. Strip file names reported in error messages on Windows, too.

  4. Fix setvbuf()-induced crash in libpq_pipeline

  5. libpq_pipeline: Must strdup(optarg) to avoid crash

  6. Remove setvbuf() call from PQtrace()

  7. Initialize conn->Pfdebug to NULL when creating a connection

  8. Disable force_parallel_mode in libpq_pipeline

  9. libpq_pipeline: add PQtrace() support and tests

  10. Improve PQtrace() output format

  11. Re-simplify management of inStart in pqParseInput3's subroutines.