Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Amit Kapila
On Wed, Jul 8, 2020 at 7:28 AM David Rowley wrote: > > > I'd really like to see this thread move forward to a solution and I'm > not sure how best to do that. I started by reading back over both this > thread and the original one and tried to summarise what people have > suggested. > Thanks, I

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread David Rowley
On Wed, 8 Jul 2020 at 07:25, Peter Geoghegan wrote: > > On Tue, Jul 7, 2020 at 5:55 AM David Rowley wrote: > > We're certainly not > > going to get that for PG13, so I do think what we need here is just a > > simple escape hatch. I mentioned my thoughts in [2], so won't go over > > it again

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Peter Geoghegan
On Tue, Jul 7, 2020 at 10:12 AM Andres Freund wrote: > I think it makes no too much sense to plan invent something like > hash_mem for v13, it's clearly too much work. That's a seperate > discussion from having something like it for v14. Can you explain why you believe that to be the case? It

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Alvaro Herrera
On 2020-Jul-07, Peter Geoghegan wrote: > On Tue, Jul 7, 2020 at 1:18 PM Alvaro Herrera > wrote: > > Yeah, backporting GUCs is not a big deal. Sure, the GUC won't appear in > > postgresql.conf files generated by initdb prior to the release that > > introduces it. But users that need it can

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Peter Geoghegan
On Tue, Jul 7, 2020 at 1:18 PM Alvaro Herrera wrote: > Yeah, backporting GUCs is not a big deal. Sure, the GUC won't appear in > postgresql.conf files generated by initdb prior to the release that > introduces it. But users that need it can just edit their .confs and > add the appropriate line,

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Alvaro Herrera
On 2020-Jul-07, Amit Kapila wrote: > I don't think this is true. We seem to have introduced three new guc > variables in a 9.3.3 minor release. Yeah, backporting GUCs is not a big deal. Sure, the GUC won't appear in postgresql.conf files generated by initdb prior to the release that introduces

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Peter Geoghegan
On Tue, Jul 7, 2020 at 5:55 AM David Rowley wrote: > FWIW, I'm not a fan of the hash_mem idea. It was my impression that we > aimed to provide an escape hatch for people we have become accustomed > to <= PG12 behaviour and hash_mem sounds like it's not that. The exact scope of the problem is

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Andres Freund
Hi, On 2020-07-03 10:08:08 -0400, Bruce Momjian wrote: > Well, the bottom line is that we are designing features during beta. > People are supposed to be testing PG 13 behavior during beta, including > optimizer behavior. I think it makes no too much sense to plan invent something like hash_mem

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Pavel Stehule
Ășt 7. 7. 2020 v 14:55 odesĂ­latel David Rowley napsal: > On Tue, 7 Jul 2020 at 16:57, Jeff Davis wrote: > > > > On Sun, 2020-07-05 at 16:47 -0700, Peter Geoghegan wrote: > > > Where does that leave the hash_mem idea (or some other similar > > > proposal)? > > > > hash_mem is acceptable to me if

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread David Rowley
On Tue, 7 Jul 2020 at 16:57, Jeff Davis wrote: > > On Sun, 2020-07-05 at 16:47 -0700, Peter Geoghegan wrote: > > Where does that leave the hash_mem idea (or some other similar > > proposal)? > > hash_mem is acceptable to me if the consensus is moving toward that, > but I'm not excited about it.

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-07 Thread Amit Kapila
On Tue, Jul 7, 2020 at 9:18 AM Jeff Davis wrote: > > On Mon, 2020-07-06 at 15:59 +0530, Amit Kapila wrote: > > I agree that it's good to wait for actual problems. But the > > > challenge > > > is that we can't backport an added GUC. > > > > > > > Is it because we won't be able to edit existing

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-06 Thread Justin Pryzby
On Mon, Jul 06, 2020 at 09:57:08PM -0700, Jeff Davis wrote: > > I think that we should offer something like hash_mem that can work as > > a multiple of work_mem, for the reason that Justin mentioned > > recently. > > This can be justified as something that more or less maintains some > > kind of

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-06 Thread Jeff Davis
On Sun, 2020-07-05 at 16:47 -0700, Peter Geoghegan wrote: > Where does that leave the hash_mem idea (or some other similar > proposal)? hash_mem is acceptable to me if the consensus is moving toward that, but I'm not excited about it. It would be one thing if hash_mem was a nice clean solution.

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-06 Thread Jeff Davis
On Mon, 2020-07-06 at 15:59 +0530, Amit Kapila wrote: > I agree that it's good to wait for actual problems. But the > > challenge > > is that we can't backport an added GUC. > > > > Is it because we won't be able to edit existing postgresql.conf file > or for some other reasons? Perhaps "can't"

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-06 Thread Amit Kapila
On Sun, Jul 5, 2020 at 2:24 AM Jeff Davis wrote: > > On Sat, 2020-07-04 at 14:49 +0530, Amit Kapila wrote: > > > We don't even have a user report yet of a > > > regression compared to PG 12, or one that can't be fixed by > > > increasing > > > work_mem. > > > > > > > Yeah, this is exactly the

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-05 Thread Peter Geoghegan
On Sat, Jul 4, 2020 at 1:54 PM Jeff Davis wrote: > I agree that it's good to wait for actual problems. But the challenge > is that we can't backport an added GUC. Are there other, backportable > changes we could potentially make if a lot of users have a problem with > v13 after release? I doubt

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-04 Thread Jeff Davis
On Sat, 2020-07-04 at 14:49 +0530, Amit Kapila wrote: > > We don't even have a user report yet of a > > regression compared to PG 12, or one that can't be fixed by > > increasing > > work_mem. > > > > Yeah, this is exactly the same point I have raised above. I feel we > should wait before

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-04 Thread Amit Kapila
On Fri, Jul 3, 2020 at 7:38 PM Bruce Momjian wrote: > > On Thu, Jul 2, 2020 at 08:35:40PM -0700, Peter Geoghegan wrote: > > But the problem isn't really the hashaggs-that-spill patch itself. > > Rather, the problem is the way that work_mem is supposed to behave in > > general, and the impact

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-03 Thread Alvaro Herrera
On 2020-Jul-03, Bruce Momjian wrote: > Well, the bottom line is that we are designing features during beta. Well, we're designing a way for users to interact the new feature. The feature itself is already in, and it works well in general terms. I expect that the new behavior is a win in the

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-03 Thread Justin Pryzby
On Fri, Jul 03, 2020 at 10:08:08AM -0400, Bruce Momjian wrote: > On Thu, Jul 2, 2020 at 08:35:40PM -0700, Peter Geoghegan wrote: > > But the problem isn't really the hashaggs-that-spill patch itself. > > Rather, the problem is the way that work_mem is supposed to behave in > > general, and the

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-03 Thread Bruce Momjian
On Thu, Jul 2, 2020 at 08:35:40PM -0700, Peter Geoghegan wrote: > But the problem isn't really the hashaggs-that-spill patch itself. > Rather, the problem is the way that work_mem is supposed to behave in > general, and the impact that that has on hash aggregate now that it > has finally been

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-02 Thread Peter Geoghegan
On Thu, Jul 2, 2020 at 7:46 PM Justin Pryzby wrote: > Thanks for putting it together, I agree that hash_mem seems to be an obvious > "escape hatch" that generalizes existing GUCs and independently useful. It is independently useful. It's a natural consequence of "being honest" about work_mem and

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-02 Thread Peter Geoghegan
On Thu, Jul 2, 2020 at 8:00 PM Bruce Momjian wrote: > Also, I feel this is all out of scope for PG 13, frankly. I think our > only option is to revert the hash spill entirely, and return to PG 13 > behavior, if people are too worried about hash performance regression. But the problem isn't

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-02 Thread Bruce Momjian
On Thu, Jul 2, 2020 at 10:58:34PM -0400, Bruce Momjian wrote: > On Thu, Jul 2, 2020 at 09:46:49PM -0500, Justin Pryzby wrote: > > On Thu, Jul 02, 2020 at 07:05:48PM -0700, Peter Geoghegan wrote: > > > anything else). I still think that the new GUC should work as a > > > multiplier of work_mem,

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-02 Thread Bruce Momjian
On Thu, Jul 2, 2020 at 09:46:49PM -0500, Justin Pryzby wrote: > On Thu, Jul 02, 2020 at 07:05:48PM -0700, Peter Geoghegan wrote: > > anything else). I still think that the new GUC should work as a > > multiplier of work_mem, or something else along those lines, though > > for now it's just an

Re: Default setting for enable_hashagg_disk (hash_mem)

2020-07-02 Thread Justin Pryzby
On Thu, Jul 02, 2020 at 07:05:48PM -0700, Peter Geoghegan wrote: > On Mon, Jun 29, 2020 at 2:46 PM Peter Geoghegan wrote: > > On Mon, Jun 29, 2020 at 2:22 PM Tomas Vondra > > wrote: > > > I agree with this, and I'm mostly OK with having hash_mem. In fact, from > > > the proposals in this thread