Date: Fri, 10 Jul 2020 10:17:14 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Peter Geoghegan <pg@bowt.ie>
Cc: 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: <20200710141714.GI12375@tamriel.snowman.net>
References: <85F9F4F0-918A-457F-BC9E-F2AC2EFC1F86@anarazel.de>
 <20200625225853.GA11137@alvherre.pgsql>
 <20200708140037.GI3125@tamriel.snowman.net>
 <99b047386525b87df1e611335c2e57a6cdef346b.camel@j-davis.com>
 <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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="cVp8NMj01v+Em8Se"
Content-Disposition: inline
In-Reply-To: 
 <CAH2-Wz=9f-gxPmqL9Lak0xddPmH0bgfAEioK+2BwTkuzyrtZEQ@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


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

Greetings,

* Peter Geoghegan (pg@bowt.ie) wrote:
> On Thu, Jul 9, 2020 at 5:08 PM Stephen Frost <sfrost@snowman.net> wrote:
> > I didn't, and don't, think it particularly relevant to the discussion,
> > but if you don't like the comparison to Sort then we could compare it to
> > a HashJoin instead- the point is that, yes, if you are willing to give
> > more memory to a given operation, it's going to go faster, and the way
> > users tell us that they'd like the query to use more memory is already
> > well-defined and understood to be through work_mem.  We do not do them a
> > service by ignoring that.
>=20
> The hash_mem design (as it stands) would affect both hash join and
> hash aggregate. I believe that it makes most sense to have hash-based
> nodes under the control of a single GUC. I believe that this
> granularity will cause the least problems. It certainly represents a
> trade-off.

So, now this has moved from being a hack to deal with a possible
regression for a small number of users due to new behavior in one node,
to a change that has impacts on other nodes that hadn't been changed,
all happening during beta.

No, I don't agree with this.  Now is not the time for designing new
features for v13.

> work_mem is less of a problem with hash join, primarily because join
> estimates are usually a lot better than grouping estimates. But it is
> nevertheless something that it makes sense to put in the same
> conceptual bucket as hash aggregate, pending a future root and branch
> redesign of work_mem.

I'm still not thrilled with the 'hash_mem' kind of idea as it's going in
the wrong direction because what's actually needed is a way to properly
consider and track overall memory usage- a redesign of work_mem (or some
new parameter, but it wouldn't be 'hash_mem') as you say, but all of
this discussion should be targeting v14.

> Like I said, the escape hatch GUC is not my preferred solution. But at
> least it acknowledges the problem. I don't think that anyone (or
> anyone else) believes that work_mem doesn't have serious limitations.

work_mem obviously has serious limitations, but that's not novel or new
or unexpected by anyone.

> > We have a parameter which already drives this and which users are
> > welcome to (and quite often do) tune.  I disagree that anything further
> > is either essential or particularly desirable.
>=20
> This is a user hostile attitude.

I don't find that argument convincing, at all.

> > I'm really rather astounded at the direction this has been going in.
>=20
> Why?

Due to the fact that we're in beta and now is not the time to be
redesigning this feature.  What Jeff implemented was done in a way that
follows the existing structure for how all of the other nodes work and
how HashAgg was *intended* to work (as in- if we thought the HashAgg
would go over work_mem, we wouldn't pick it and would do a GroupAgg
instead).  If there's bugs in his implementation (which I doubt, but it
can happen, of course) then that'd be useful to discuss and look at
fixing, but this discussion isn't appropriate for beta.

Thanks,

Stephen

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

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

iQIcBAEBCgAGBQJfCHhqAAoJEO1sijiDR2RVzCMQAI84WTT4KZ69UFKDSZ9gioNm
gnDB/kICZ7OrHJjlJaAm/FVRtws9TixeBD7W3fT1SLLS/ZIQKQZge+mPJYejaG3C
OEYJXTFNA6OmAoqAWmqY7zpPgpLplHNGJfxMR8nr+0nf6+aLwQA+c5Sbf1QVkA00
tFP1oADL/hv881aRhrghwjfLalpWcxizEdPRFSA2q1s8I8pnn6H31h281MOguUhs
7gEr9QhEAjud4PDotNrdyJONrgqDRcOlxAkYtanutP28eDV0W13sbVG5drUTCWGb
ijFsbMhwJAygC78gdJB+YjQHgt+W/IP3pA2Pylqu9R9wPCoHnaiA2SsZl8fFTK2l
1SVxwZGeE3OIhyihIgToYaRPFNf+X0jHzjaanU90JiretrojZFkdi+vn9z5VakOn
SZWcdOWaB1lEfMPPp0Ml5ely9FjbNkmVkkzXSHZm9ijsFCavRSB1MjdCMmD7iyBT
LWxjiTLBymcqaWvUF2NUAXMDOB6vtEH6zl8wNiX4gm09Q6utlfGm4+HjsjcKVew9
/41QHRj1RnJL6gpE7mN2UC7+ysIqVBWK0EYpn+2Koik1xeyvK55h1YhuLdGUgJ+5
E9paqgahnSYVe+Zzb8YGIBBgsa75zui6KsavI6yv3179Aklstmos9LDLWntoVDpH
H12l5Wq6hOJZ6jbwTk0y
=Q+c2
-----END PGP SIGNATURE-----

--cVp8NMj01v+Em8Se--