Greetings, On Fri, Apr 8, 2022 at 07:27 Magnus Hagander <mag...@hagander.net> wrote:
> On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander <mag...@hagander.net> > wrote: > >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley <dgrowle...@gmail.com> >> wrote: >> >>> On Wed, 30 Mar 2022 at 02:38, Robert Haas <robertmh...@gmail.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 >
