Date: Sat, 11 Jul 2020 10:22:13 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Geoghegan <pg@bowt.ie>, Jeff Davis <pgsql@j-davis.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Andres Freund <andres@anarazel.de>,
	Bruce Momjian <bruce@momjian.us>,
	Robert Haas <robertmhaas@gmail.com>,
	David Rowley <dgrowleyml@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Melanie Plageman <melanieplageman@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Default setting for enable_hashagg_disk
Message-ID: <20200711142213.GP12375@tamriel.snowman.net>
References: <20200709140330.GO3125@tamriel.snowman.net>
 <CAH2-WzmB04dxgRgjDGvDbugEFfHxMi2pyuJqsYr30x1RAEqSXg@mail.gmail.com>
 <20200709225840.GE12375@tamriel.snowman.net>
 <CAH2-WzmbycnZnxFTYnwkSkCPTVQbRoLiuEvo8gkZCKiQTL8GAQ@mail.gmail.com>
 <20200710000842.GF12375@tamriel.snowman.net>
 <CAH2-Wz=9f-gxPmqL9Lak0xddPmH0bgfAEioK+2BwTkuzyrtZEQ@mail.gmail.com>
 <20200710141714.GI12375@tamriel.snowman.net>
 <CAH2-Wzkd7cWK-mMPMb63JXxj2QFMan6hw+cQjdJKvdtCOpR7Ng@mail.gmail.com>
 <20200710215012.GN12375@tamriel.snowman.net>
 <2165606.1594418399@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="AwNVUpjOmSj7UnwZ"
Content-Disposition: inline
In-Reply-To: <2165606.1594418399@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


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

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > I don't see hash_mem as being any kind of proper fix- it's just punting
> > to the user saying "we can't figure this out, how about you do it" and,
> > worse, it's in conflict with how we already ask the user that question.
> > Turning it into a multiplier doesn't change that either.
>=20
> Have you got a better proposal that is reasonably implementable for v13?
> (I do not accept the argument that "do nothing" is a better proposal.)
>=20
> I agree that hash_mem is a stopgap, whether it's a multiplier or no,
> but at this point it seems difficult to avoid inventing a stopgap.
> Getting rid of the process-global work_mem setting is a research project,
> and one I wouldn't even count on having results from for v14.  In the
> meantime, it seems dead certain that there are applications for which
> the current behavior will be problematic.  hash_mem seems like a cleaner
> and more useful stopgap than the "escape hatch" approach, at least to me.

Have we heard from people running actual applications where there is a
problem with raising work_mem to simply match what's already happening
with the v12 behavior?

Sure, there's been some examples on this thread of people who know the
backend well showing how the default work_mem will cause the v13 HashAgg
to spill to disk when given a query which has poor estimates, and that's
slower than v12 where it ignored work_mem and used a bunch of memory,
but it was also shown that raising work_mem addresses that issue and
brings v12 and v13 back in line.

There was a concern raised that other nodes might then use more memory-
but there's nothing new there, if you wanted to avoid batching with a
HashJoin in v12 you'd have exactly the same issue, and yet folks raise
work_mem all the time to address this, and to get that HashAgg plan in
the first place too when the estimates aren't so far off.

There now seems to be some suggestions that not only should we have a
new GUC, but we should default to having it not be equal to work_mem (or
1.0 or whatever) and instead by higher, to be *twice* or larger whatever
the existing work_mem setting is- meaning that people whose systems are
working just fine and have good estimates that represent their workload
and who get the plans they want may then start seeing differences and
increased memory utilization in places that they *don't* want that, all
because we're scared that someone, somewhere, might see a regression due
to HashAgg spilling to disk.

So, no, I don't agree that 'do nothing' (except ripping out the one GUC
that was already added) is a worse proposal than adding another work_mem
like thing that's only for some nodes types.  There's no way that we'd
even be considering such an approach during the regular development
cycle either- there would be calls for a proper wholistic view, at least
to the point where every node type that could possibly allocate a
reasonable chunk of memory would be covered.

Thanks,

Stephen

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

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

iQIcBAEBCgAGBQJfCcsVAAoJEO1sijiDR2RVbPAP/3axvKrYET8hq7wEbs3w+C1G
rfHTiR50XMW3uoM8CLQzqO/p0ThMQ+p3rhCuk3x/6zYeIX3THjMJJplzL/D20nY4
p67hahgBr+5m08rmg/HGYuDXx4Ga+sDgWBPGFlNcc1IV2Y45uxOwacpe6/bQEOg6
7xzcYCo166iWpdWRQbK2sylEnikgpoInBGfB+WQ+VJ8RIcepbaY5u487CFc+UtJg
Vou5prvj4Djk11NhnO+QFkXRfZ0YVuN1mBUgx5Rjpfezhm1q3St6EKOhUS+HcPuh
nuVnucBEAbKKRHdVR8Vc1jNwm/45W+khGQYcXEb7VvDqxC9H6XcPoLtEak4cbMlJ
dGXIvNnd/mZm7wtsAu+3r65Ya7NrLRHwGGKNvgOBB1J9xIWUQBOP54xuMiy8Fhf+
KvThDBIlcT7GMlZji9Id0lUtlqQQDYnYqPOlQySqy0i1ykC8zy7iREed3gpn+uCD
EhPHEH4dngUkwxXcY1hHW7LZxJsYrwPQ1UZCSw/zc3SIMAiNg2a7sgeciuMfqnfI
lG77fyCYARGimVWqnYJRmykUbEJw8mnAGzyS59tNXa8jF6ej9zNpdvpK7+sQhBJf
8K6CiBv6bDWEBgLyy7uQbpNdyVCmTimj+/aHhV/Ftz5TtauQBJJzudaemG5RLGwI
yqT3J3mTiSnSe9qCJiVN
=Ok0h
-----END PGP SIGNATURE-----

--AwNVUpjOmSj7UnwZ--