Re: Valgrind Memcheck support

On 2013-08-27 23:46:23 -0400, Noah Misch wrote:
> On Tue, Aug 27, 2013 at 04:14:27PM +0200, Andres Freund wrote:
> > On 2013-06-09 17:25:59 -0400, Noah Misch wrote:
> > > ***************
> > > *** 846,851 **** exec_simple_query(const char *query_string)
> > > --- 847,856 ----
> > >
> > > TRACE_POSTGRESQL_QUERY_START(query_string);
> > >
> > > + #ifdef USE_VALGRIND
> > > + VALGRIND_PRINTF("statement: %s\n", query_string);
> > > + #endif
> > > +
> >
> > Is there a special reason for adding more logging here? I find this
> > makes the instrumentation much less useful since reports easily get
> > burried in those traces. What's the advantage of doing this instead of
> > log_statement=...? Especially as that location afaics won't help for the
> > extended protocol?
>
> I typically used "valgrind --log-file=...". To determine via log_statement
> which SQL statement caused a particular Valgrind error, I would match by
> timestamp; this was easier. In retrospect, log_statement would have sufficed
> given both Valgrind and PostgreSQL logging to stderr. Emitting the message in
> exec_simple_query() and not in exec_execute_message() is indeed indefensible.

It's an understandable mistake, nothing indefensible. We don't really
test the extended protocol :(

> That being said, could you tell me more about your workflow where the extra
> messages cause a problem? Do you just typically diagnose each Valgrind error
> without first isolating the pertinent SQL statement?

There's basically two scenarios where I found it annoying:
a) I manually reproduce some issue, potentially involving several psql
shells. All those fire up autocompletion and such queries which bloat
the log while I actually only want to analyze something specific.
b) I don't actually look for anything triggered by an SQL statement
itself, but am looking for issues in a walsender, background worker,
whatever. I still need some foreground activity to trigger the behaviour
I want to see, but once more, valgrind's logs hide the error.

Also, I sometimes use valgrind's --db-command/--vgdb which gives you
enough context from the backtrace itself since you can just get the
statement from there.

I vote for just removing that VALGRIND_PRINTF - it doesn't give you
anything you cannot easily achieve otherwise...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services