Re: Add parameter jit_warn_above_fraction

2022-09-15 Thread Ibrar Ahmed
On Sat, Apr 9, 2022 at 8:43 PM Julien Rouhaud wrote: > On Sat, Apr 09, 2022 at 12:31:23PM -0400, Tom Lane wrote: > > Julien Rouhaud writes: > > > On Sat, Apr 09, 2022 at 10:42:12AM -0400, Tom Lane wrote: > > >> Also, good luck with "looking in the logs", because by default > > >> WARNING-level m

Re: Add parameter jit_warn_above_fraction

2022-04-09 Thread Julien Rouhaud
On Sat, Apr 09, 2022 at 12:31:23PM -0400, Tom Lane wrote: > Julien Rouhaud writes: > > On Sat, Apr 09, 2022 at 10:42:12AM -0400, Tom Lane wrote: > >> Also, good luck with "looking in the logs", because by default > >> WARNING-level messages don't go to the postmaster log. > > > I must be missing

Re: Add parameter jit_warn_above_fraction

2022-04-09 Thread Tom Lane
Julien Rouhaud writes: > On Sat, Apr 09, 2022 at 10:42:12AM -0400, Tom Lane wrote: >> Also, good luck with "looking in the logs", because by default >> WARNING-level messages don't go to the postmaster log. > I must be missing something because by default log_min_messages is WARNING, > and > AFA

Re: Add parameter jit_warn_above_fraction

2022-04-09 Thread Julien Rouhaud
On Sat, Apr 09, 2022 at 10:42:12AM -0400, Tom Lane wrote: > > Also, good luck with "looking in the logs", because by default > WARNING-level messages don't go to the postmaster log. If that's > the intended use-case then the message ought to appear at LOG > level (which'd also change the desirable

Re: Add parameter jit_warn_above_fraction

2022-04-09 Thread Tom Lane
Julien Rouhaud writes: > On Fri, Apr 08, 2022 at 09:39:18AM -0400, Stephen Frost wrote: >> Having this in pg_stat_statements is certainly helpful but having a >> warning also is. I don't think we have to address this in only one way. >> A lot faster to flip this guc and then look in the logs on a

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread Julien Rouhaud
Hi, On Fri, Apr 08, 2022 at 09:39:18AM -0400, Stephen Frost wrote: > > * Magnus Hagander (mag...@hagander.net) wrote: > > The addition to pg_stat_statements I pushed a short while ago would help > > with that. But I think having a warning like this would also be useful. As > > a stop-gap measure,

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread Stephen Frost
Greetings, * Magnus Hagander (mag...@hagander.net) wrote: > On Fri, Apr 8, 2022 at 2:19 PM David Rowley wrote: > > On Fri, 8 Apr 2022 at 23:27, Magnus Hagander wrote: > > > On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander > > wrote: > > >> > > >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley >

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread Magnus Hagander
On Fri, Apr 8, 2022 at 2:19 PM David Rowley wrote: > On Fri, 8 Apr 2022 at 23:27, Magnus Hagander wrote: > > > > > > > > On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander > wrote: > >> > >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley > wrote: > >>> > >>> If we go with this patch, the problem

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread David Rowley
On Fri, 8 Apr 2022 at 23:27, Magnus Hagander wrote: > > > > On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander wrote: >> >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley wrote: >>> >>> If we go with this patch, the problem I see here is that the amount >>> of work the JIT compiler must do for a gi

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread Stephen Frost
Greetings, On Fri, Apr 8, 2022 at 07:27 Magnus Hagander wrote: > On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander > wrote: > >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley >> wrote: >> >>> On Wed, 30 Mar 2022 at 02:38, Robert Haas wrote: >>> > I think WARNING is fine. After all, the paramete

Re: Add parameter jit_warn_above_fraction

2022-04-08 Thread Magnus Hagander
On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander wrote: > On Tue, Mar 29, 2022 at 10:06 PM David Rowley > wrote: > >> On Wed, 30 Mar 2022 at 02:38, Robert Haas wrote: >> > I think WARNING is fine. After all, the parameter is called >> > "jit_warn_above_fraction". >> >> I had a think about this p

Re: Add parameter jit_warn_above_fraction

2022-03-30 Thread Magnus Hagander
On Tue, Mar 29, 2022 at 10:06 PM David Rowley wrote: > On Wed, 30 Mar 2022 at 02:38, Robert Haas wrote: > > I think WARNING is fine. After all, the parameter is called > > "jit_warn_above_fraction". > > I had a think about this patch. I guess it's a little similar to > checkpoint_warning. The g

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread David Rowley
On Wed, 30 Mar 2022 at 14:48, Andres Freund wrote: > > On 2022-03-30 14:30:32 +1300, David Rowley wrote: > > Maybe nodes below an Append/MergeAppend with run-time pruning could compile > > on-demand and other nodes up-front. Or maybe there's no problem with making > > everything on-demand. > > Ye

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Andres Freund
Hi, On 2022-03-30 14:30:32 +1300, David Rowley wrote: > On Wed, 30 Mar 2022 at 13:20, Andres Freund wrote: > > I wonder whether it'd make sense to combine that with awareness of a few > > plan > > types that can lead to large portions of child nodes never being executed. > > One > > the case wh

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread David Rowley
On Wed, 30 Mar 2022 at 13:20, Andres Freund wrote: > I wonder whether it'd make sense to combine that with awareness of a few plan > types that can lead to large portions of child nodes never being executed. One > the case where the current behaviour is the worst is runtime partition pruning > in

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Andres Freund
Hi, On 2022-03-30 12:41:41 +1300, David Rowley wrote: > On Wed, 30 Mar 2022 at 12:16, Andres Freund wrote: > > > I did propose a patch to address this in [1]. It does need more work > > > and I do plan to come back to it for v16. > > > > FWIW, that doesn't seem quite right - won't it stop JITing

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Tom Lane
David Rowley writes: > On Wed, 30 Mar 2022 at 12:16, Andres Freund wrote: >> FWIW, that doesn't seem quite right - won't it stop JITing e.g. on the inner >> side of a nested loop, just because it's cheap, even though that's where the >> bulk of the benefits comes from? > Yeah, I think the total

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread David Rowley
On Wed, 30 Mar 2022 at 12:16, Andres Freund wrote: > > I did propose a patch to address this in [1]. It does need more work > > and I do plan to come back to it for v16. > > FWIW, that doesn't seem quite right - won't it stop JITing e.g. on the inner > side of a nested loop, just because it's chea

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Andres Freund
Hi, On 2022-03-30 09:05:44 +1300, David Rowley wrote: > I really believe that the main problem here is that JIT only enables > when the *total* plan cost reaches a certain threshold. Yes, that is/was a clear design mistake. It wasn't quite as bad back when it was written - partitioning blows the

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Peter Geoghegan
On Tue, Mar 29, 2022 at 3:04 PM Tom Lane wrote: > I think David's questions are sufficiently cogent and difficult > that we should not add jit_warn_above_fraction at this time. +1 -- Peter Geoghegan

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Tom Lane
Peter Geoghegan writes: > On Tue, Mar 29, 2022 at 1:06 PM David Rowley wrote: >> That means that even if the total execution time of a plan was a true >> reflection of the total estimated plan cost, then the fraction of time >> spent (as is measured by jit_warn_above_fraction) doing JIT would >>

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Peter Geoghegan
On Tue, Mar 29, 2022 at 1:06 PM David Rowley wrote: > That means that even if the total execution time of a plan was a true > reflection of the total estimated plan cost, then the fraction of time > spent (as is measured by jit_warn_above_fraction) doing JIT would > entirely depend on the number o

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread David Rowley
On Wed, 30 Mar 2022 at 02:38, Robert Haas wrote: > I think WARNING is fine. After all, the parameter is called > "jit_warn_above_fraction". I had a think about this patch. I guess it's a little similar to checkpoint_warning. The good thing about the checkpoint_warning is that in the LOG message

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Robert Haas
On Tue, Mar 29, 2022 at 6:09 AM Julien Rouhaud wrote: > The last remaining thing is whether logging at WARNING level is the correct > choice. I'm personally fine with it, because the people who are going to use > it will probably use the same approach as for log_min_duration_statements: > enable

Re: Add parameter jit_warn_above_fraction

2022-03-29 Thread Julien Rouhaud
Hi, On Mon, Mar 28, 2022 at 10:11:16PM +0200, Magnus Hagander wrote: > On Tue, Mar 22, 2022 at 12:50 AM Andres Freund wrote: > > > > This fails on cfbot, due to compiler warnings: > > https://cirrus-ci.com/task/5127667648299008?logs=mingw_cross_warning#L390 > > > Huh. That's annoying. I forgot in

Re: Add parameter jit_warn_above_fraction

2022-03-28 Thread Magnus Hagander
On Tue, Mar 22, 2022 at 12:50 AM Andres Freund wrote: > Hi, > > On 2022-03-07 13:10:32 +0100, Magnus Hagander wrote: > > Meanwhile here is an updated based on your other comments above, as > > well as those from Julien. > > This fails on cfbot, due to compiler warnings: > https://cirrus-ci.com/ta

Re: Add parameter jit_warn_above_fraction

2022-03-21 Thread Andres Freund
Hi, On 2022-03-07 13:10:32 +0100, Magnus Hagander wrote: > Meanwhile here is an updated based on your other comments above, as > well as those from Julien. This fails on cfbot, due to compiler warnings: https://cirrus-ci.com/task/5127667648299008?logs=mingw_cross_warning#L390 Greetings, Andres

Re: Add parameter jit_warn_above_fraction

2022-03-12 Thread Justin Pryzby
On Mon, Mar 07, 2022 at 04:02:16PM +0100, Magnus Hagander wrote: > On Mon, Mar 7, 2022 at 2:09 PM Justin Pryzby 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 > > > wrote: > > > > > > > > I think it should be a NOTI

Re: Add parameter jit_warn_above_fraction

2022-03-07 Thread Magnus Hagander
On Mon, Mar 7, 2022 at 2:09 PM Justin Pryzby 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 wrote: > > > > > > I think it should be a NOTICE (or less?) > > > > Hmm. I'm not so sure. > > > > Other similar parameters oft

Re: Add parameter jit_warn_above_fraction

2022-03-07 Thread Justin Pryzby
On Mon, Mar 07, 2022 at 01:10:32PM +0100, Magnus Hagander wrote: > On Fri, Feb 25, 2022 at 5:23 PM Justin Pryzby 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 th

Re: Add parameter jit_warn_above_fraction

2022-03-07 Thread Magnus Hagander
On Fri, Feb 25, 2022 at 5:23 PM Justin Pryzby wrote: > > On Fri, Feb 25, 2022 at 04:16:01PM +0100, Magnus Hagander wrote: > > + { > > + {"jit_warn_above_fraction", PGC_SUSET, LOGGING_WHEN, > > + gettext_noop("Sets the fraction of query time spent > > on JIT bef

Re: Add parameter jit_warn_above_fraction

2022-03-07 Thread Magnus Hagander
On Fri, Feb 25, 2022 at 5:47 PM Andres Freund wrote: > > Hi, > > On 2022-02-25 17:28:41 +0100, Magnus Hagander wrote: > > On Fri, Feb 25, 2022 at 5:20 PM Andres Freund wrote: > > > On 2022-02-25 16:16:01 +0100, Magnus Hagander wrote: > > > > This patch adds a configuration parameter jit_warn_abov

Re: Add parameter jit_warn_above_fraction

2022-02-25 Thread Andres Freund
Hi, On 2022-02-25 17:28:41 +0100, Magnus Hagander wrote: > On Fri, Feb 25, 2022 at 5:20 PM Andres Freund wrote: > > On 2022-02-25 16:16:01 +0100, Magnus Hagander wrote: > > > This patch adds a configuration parameter jit_warn_above_fraction that > > > will cause a warning to be logged if the frac

Re: Add parameter jit_warn_above_fraction

2022-02-25 Thread Magnus Hagander
On Fri, Feb 25, 2022 at 5:20 PM Andres Freund wrote: > > Hi, > > On 2022-02-25 16:16:01 +0100, Magnus Hagander wrote: > > This patch adds a configuration parameter jit_warn_above_fraction that > > will cause a warning to be logged if the fraction of time spent on > > doing JIT is bigger than the s

Re: Add parameter jit_warn_above_fraction

2022-02-25 Thread Justin Pryzby
On Fri, Feb 25, 2022 at 04:16:01PM +0100, Magnus Hagander wrote: > + { > + {"jit_warn_above_fraction", PGC_SUSET, LOGGING_WHEN, > + gettext_noop("Sets the fraction of query time spent on > JIT before writing" > + "a w

Re: Add parameter jit_warn_above_fraction

2022-02-25 Thread Andres Freund
Hi, On 2022-02-25 16:16:01 +0100, Magnus Hagander wrote: > This patch adds a configuration parameter jit_warn_above_fraction that > will cause a warning to be logged if the fraction of time spent on > doing JIT is bigger than the specified one. For example, this can be > used to track down those c

Re: Add parameter jit_warn_above_fraction

2022-02-25 Thread Julien Rouhaud
Hi, On Fri, Feb 25, 2022 at 04:16:01PM +0100, Magnus Hagander wrote: > This patch adds a configuration parameter jit_warn_above_fraction that > will cause a warning to be logged if the fraction of time spent on > doing JIT is bigger than the specified one. For example, this can be > used to track