Date: Fri, 8 Apr 2022 09:39:18 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Magnus Hagander <magnus@hagander.net>
Cc: David Rowley <dgrowleyml@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Julien Rouhaud <rjuju123@gmail.com>,
	Andres Freund <andres@anarazel.de>,
	Justin Pryzby <pryzby@telsasoft.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add parameter jit_warn_above_fraction
Message-ID: <20220408133918.GE10577@tamriel.snowman.net>
References: <CABUevExe1fV0RzjA7vQRp_WkjtTVF1O9E5STKodWa2CmpFoqHA@mail.gmail.com>
 <20220321235048.wb3ymun6lbi33b2u@alap3.anarazel.de>
 <CABUevEy9H+HPAuM1ofC=OuY2+bESOsA4hHSAY6HbLYyTG1oaPw@mail.gmail.com>
 <20220329100846.3iedk5ear3ewdi5j@jrouhaud>
 <CA+TgmoZ-AoXM=J5J===maHDY9O+Mbz-=Sfz-A23ZbiJoMKe_Rg@mail.gmail.com>
 <CAApHDvrEoQ5p61NjDCKVgEWaH0qm1KprYw2-7m8-6ZGGJ8A2Dw@mail.gmail.com>
 <CABUevExkr8pajkKFD=OnTj3q0S0GZc-u6iqD6L0ZXjO8z3-1gQ@mail.gmail.com>
 <CABUevEw+dC=6sieitWSqqEjz8OaACOihxpWempoLewdKGnnrig@mail.gmail.com>
 <CAApHDvor25wyUUPm6vAZn8uDNaQ=15fDh+4XuBGo3-_QCmfZUg@mail.gmail.com>
 <CABUevEwT+nMj+m+=z=wPyYzq3Mh9ohuqrWBjR7q9uZawBA6j3A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="R29N3WMYO0Bgd5T+"
Content-Disposition: inline
In-Reply-To: <CABUevEwT+nMj+m+=z=wPyYzq3Mh9ohuqrWBjR7q9uZawBA6j3A@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: <https://www.postgresql.org/message-id/20220408133918.GE10577%40tamriel.snowman.net>
Precedence: bulk


--R29N3WMYO0Bgd5T+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Magnus Hagander (magnus@hagander.net) wrote:
> On Fri, Apr 8, 2022 at 2:19 PM David Rowley <dgrowleyml@gmail.com> wrote:
> > On Fri, 8 Apr 2022 at 23:27, Magnus Hagander <magnus@hagander.net> wrot=
e:
> > > On Wed, Mar 30, 2022 at 3:04 PM Magnus Hagander <magnus@hagander.net>
> > wrote:
> > >>
> > >> On Tue, Mar 29, 2022 at 10:06 PM David Rowley <dgrowleyml@gmail.com>
> > 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 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 h=
ave
> > >>> 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 lik=
ely
> > >>> 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 m=
ean
> > you "rarely JIT anything atll", it just means you rarely JIT that
> > particular query.
> >
> > I just struggle to imagine that anyone is going to spend much effort
> > tuning a warning parameter per query.  I imagine they're far more
> > likely to just ramp it up to only catch some high percentile problems
> > or just (more likely) just not bother with it.  It seems more likely
> > that if anyone was to tune anything per query here it would be
> > jit_above_cost, since that actually might have an affect on the
> > performance of the query, rather than if it spits out some warning
> > message or not.  ISTM that if the user knows what to set it to per
> > query, then there's very little point in having a warning as we'd be
> > alerting them to something they already know about.
>=20
> I would not expect people to tune the *warning* at a query level. If
> anything, then ys, they would tune the either jit_above_cost or just
> jit=3Doff. But the idea being you can do that on a per query level instea=
d of
> globally.

Yeah, exactly, this is about having a busy system and wanting to know
which queries are spending a lot of time doing JIT relative to the query
time, so that you can go adjust your JIT parameters or possibly disable
JIT for those queries (or maybe bring those cases to -hackers and try to
help make our costing better).

> > I looked in the -general list to see if we could get some common
> > explanations to give us an idea of the most common reason for high JIT
> > compilation time. It seems that the plans were never simple. [1] seems
> > due to a complex plan. I'm basing that off the "Functions: 167". I
> > didn't catch the full plan. From what I can tell, [2] seems to be due
> > to "lots of empty tables", so assuming the clamping at 1 page is
> > causing issues there.  I think both of those cases could be resolved
> > by building the costing the way I mentioned.  I admit that 2 cases is
> > not a very large sample size.
>=20
> Again, I am very much for improvements of the costing model. This is in no
> way intended to be a replacement for that. It's intended to be a stop-gap.

Not sure I'd say it's a 'stop-gap' as it's really very similar, imv
anyway, to log_min_duration_statement- you want to know what queries are
taking a lot of time but you can't log all of them.

> What I see much of today are things like
> https://dba.stackexchange.com/questions/264955/handling-performance-probl=
ems-with-jit-in-postgres-12
> or
> https://dev.to/xenatisch/cascade-of-doom-jit-and-how-a-postgres-update-le=
d-to-70-failure-on-a-critical-national-service-3f2a
>=20
> The bottom line is that people end up with recommendations to turn off JIT
> globally more or less by default. Because there's no real useful way today
> to figure out when it causes problems vs when it helps.

Yeah, that's frustrating.

> 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, yes, but we really don't know when we will have an
> improved costing model for it. I hope you're right and that we can have it
> by 16, and then I will definitely advocate for removing the warning again
> if it works.

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 busy system
than to install pg_stat_statements, restart the cluster once you get
permission to do so, and then query it.

Thanks,

Stephen

--R29N3WMYO0Bgd5T+
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJiUDsFAAoJEO1sijiDR2RVY20P/jdfNGoH8WNVpAlyTopF83u9
GH1vv9c6OPAnAuuVW+hR8lA/JmrRa6t32FgEXCSFQ/KyWULcTiCVCvezK85ySxJI
Goy0KJrk7Q2OzJh/APGZC5g/ZFtevYnOFjxO7kWTqdXeiA7YjyzZdjvlBq3l9Tg1
luiWGxgzr7rmGeoYhvxeBHZ4ibgfrcPr/s9SX3NtYUyHf0tsjrox4Iy6bZrkGUvW
MFugLvwB8WtGcnGb9pS1v5ScQyoFPx93/4MQDhyYPLmJNvBrwqE8qWqAqY2ziUyv
2YGVDrw9HCM6HxlwRR9ns5amFOwdr28oqAsjoAoT3hacn29U7QueC+W3Fvldk2Gw
b3HER2TQRwqn3vP1rd+za/tYeQL+CfUOK4PdgzOXB45gRXFvVJoSV0gOZ6IH4K87
y6zm8LliF+sPMKJc9EAH5xjDuycHgPHT5SvEJgDrWl3Ga0gaRwFOn1B3mq1wOFN1
lP9ecUZpONXXLH/2spKXHje4/4E9BgsP/qjt7aKUOHZUcrKqK3wl8fKJbD3199y6
BsgkjMstEnpKZbJAvl7p6CIIOpvDQI7WBSRJdyI50KpAiKx/0fnnjm4BbQQkbtfY
8omZRoMnDc7aaHLQ/YETXWskD1noXnq/bjgTjygZSKwx92rlLmUCt/cN4YJ9fko/
eSu71012HHd6Ti0fir0q
=LDFA
-----END PGP SIGNATURE-----

--R29N3WMYO0Bgd5T+--