Re: Add parameter jit_warn_above_fraction

Greetings,

On Fri, Apr 8, 2022 at 07:27 Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
>
>> On Tue, Mar 29, 2022 at 10:06 PM David Rowley <dgrowleyml(at)gmail(dot)com>
>> wrote:
>>
>>> On Wed, 30 Mar 2022 at 02:38, Robert Haas <robertmhaas(at)gmail(dot)com> 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 we give instructions about how the DBA can fix
>>> the issue, i.e increase max_wal_size.
>>>
>>> With the proposed patch I see there is no hint about what might be
>>> done to remove/reduce the warnings. I imagine that's because it's not
>>> all that clear which GUC should be changed. In my view, likely
>>> jit_above_cost is the most relevant but there is also
>>> jit_inline_above_cost, jit_optimize_above_cost, jit_tuple_deforming
>>> and jit_expressions which are relevant too.
>>>
>>> If we go with this patch, the problem I see here is that the amount
>>> of work the JIT compiler must do for a given query depends mostly on
>>> the number of expressions that must be compiled in the query (also to
>>> a lesser extent jit_inline_above_cost, jit_optimize_above_cost,
>>> jit_tuple_deforming and jit_expressions). The DBA does not really have
>>> much control over the number of expressions in the query. All he or
>>> she can do to get rid of the warning is something like increase
>>> jit_above_cost. After a few iterations of that, the end result is
>>> that jit_above_cost is now high enough that JIT no longer triggers
>>> for, say, that query to that table with 1000 partitions where no
>>> plan-time pruning takes place. Is that really a good thing? It likely
>>> means that we just rarely JIT anything at all!
>>>
>>
>> I don't agree with the conclusion of that.
>>
>> What the parameter would be useful for is to be able to tune those costs
>> (or just turn it off) *for that individual query*. That doesn't mean you
>> "rarely JIT anything atll", it just means you rarely JIT that particular
>> query.
>>
>> In fact, my goal is to specifically make people do that and *not* just
>> turn off JIT globally.
>>
>>
>> I'd much rather see us address the costing problem before adding some
>>> warning, especially a warning where it's not clear how to make go
>>> away.
>>>
>>
>> The easiest way would be to add a HINT that says turn off jit for this
>> particular query or something?
>>
>> I do agree that if we can make "spending too much time on JIT vs query
>> runtime" go away completely, then there is no need for a parameter like
>> this.
>>
>> I still think the warning is useful. And I think it may stay useful even
>> after we have made the JIT costing smarter -- though that's not certain of
>> course.
>>
>>
> This patch is still sitting at "ready for committer".
>
> As an example, I have added such a hint in the attached.
>
> I still stand by that this patch is better than nothing. Sure, I would
> love for us to adapt the JIT costing model and algorithm to make this not a
> problem. And once we've done that, we should remove the parameter again.
>
> It's not on by default, and it's trivial to remove in the future.
>
>
> Yes, we're right up at the deadline. I'd still like to get it in, so I'd
> really appreciate some further voices :)
>

Looks reasonable to me, so +1. The default is has it off and so I seriously
doubt it’ll cause any issues and it’ll be very handy on large and busy
systems with lots of queries finding those that have a serious amount of
time being spent in JIT (and hopefully avoid folks just turning jit off
across the board, since that’s worse- we need more data on jit and need to
work on improving it, not ending up with everyone turning it off).

Thanks!

Stephen

>