Re: Add parameter jit_warn_above_fraction - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: Add parameter jit_warn_above_fraction
Date
Msg-id CABUevEx=J3b9=OQ8A9xOy7LvBwwD5rEb1kyQJqw-8iM7uh-7KQ@mail.gmail.com
Whole thread Raw
In response to Re: Add parameter jit_warn_above_fraction  (Justin Pryzby <pryzby@telsasoft.com>)
Responses Re: Add parameter jit_warn_above_fraction  (Justin Pryzby <pryzby@telsasoft.com>)
List pgsql-hackers
On Mon, Mar 7, 2022 at 2:09 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Mon, Mar 07, 2022 at 01:10:32PM +0100, Magnus Hagander wrote:
> > On Fri, Feb 25, 2022 at 5:23 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > >
> > > I think it should be a NOTICE (or less?)
> >
> > Hmm. I'm not so sure.
> >
> > Other similar parameters often use LOG, but the downside of that is
> > that it won't be sent to the client.
> >
> > The problem with using NOTICE is that it won't go to the log by
> > default. It needs to be at least warning to do that.
>
> Anyone who uses this parameter is aleady going to be changing GUCs, so it
> doesn't need to work by default.  The people who are likely to enable this
> already monitor their logs and have probably customized their logging
> configuration.

Sure, but if you set log_min_messagse to NOTICE you are likely going
to flood your logs beyond usefulness. And the whole idea of this
parameter is you can keep it on during "normal times" to catch
outliers.


> > > Is it more useful if this is applied combined with log_min_duration_statement ?
> > >
> > > It's easy to imagine a query for which the planner computes a high cost, but
> > > actually runs quickly.  You might get a bunch of WARNINGs that the query took
> > > 10 MS and JIT was 75% of that, even if you don't care about queries that take
> > > less than 10 SEC.
> >
> > Yeah, that's definitely a problem. But which way would you want to tie
> > it to log_min_duration_statement?  That a statement would both have to
> > take longer than log_min_duration_statement *and* have JIT above a
> > certain percentage? In my experience that is instead likely to miss
> > most of the interesting times.
>
> I don't understand - why doesn't it combine trivially with
> log_min_duration_statement?  Are you saying that the default / pre-existing min
> duration may not log all of the intended queries ?  I think that just means the
> configuration needs to be changed.  The GUC needs to allow/help finding these
> JIT issues, but going to require an admin's interaction in any case.  Avoiding
> false positives may be important for it to be useful at all.
>
> Hmm .. should it also combine with min_sample_rate ?  Maybe that addresses your
> concern.

For one, what if I don't want to log any queries unless this is the
case? I leave log_min_duration_statement at -1...
Or what if I want to log say only queries taking more than 60 seconds.
Now I will miss all queries taking 30 seconds where 99.9% of the time
is spent on JIT which I definitely *would* want.

If all I wanted was to get the JIT information for all queries over a
certain amount of time, then as someone else mentioned as well, I can
just use auto_explain. The point here is I want to trigger the logging
for queries where the runtime is *shorter* than what I have in
log_min_duration_statement, but a large portion of the runtime is
spent on JIT.

I maen. If I have a bunch of queries that take 10ms and 75% is spent
in JIT (per your example above), that is actually a problem and I'd
like to know about it and fix it (by tuning the jit triggers either
globally or locally).  Yes, at 10ms I *may* ignore it, and in general
I'd use pg_stat_statements for that. But at 100ms with 75% JIT it
starts becoming exactly the situation that I intended this patch to
help with.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/



pgsql-hackers by date:

Previous
From: Nitin Jadhav
Date:
Subject: Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Next
From: Magnus Hagander
Date:
Subject: Re: New developer papercut - Makefile references INSTALL