pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feeddon't allocate HashAgg hash tables when running explain only
6+ messages / 4 participants
[nested] [flat]
* don't allocate HashAgg hash tables when running explain only
@ 2020-11-13 16:10 Alexey Bashtanov <bashtanov@imap.cc>
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Bashtanov @ 2020-11-13 16:10 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I got somewhat scared when my explain took a few seconds to complete and
used a few gigs of RAM.
To reproduce try the following:
discard temp;
create temp table a as select to_timestamp(generate_series(1, 7000)) i;
analyze a;
set work_mem to '3GB';
explain select distinct a1.i - a2.i from a a1, a a2;
I would appreciate if someone could have a look at the patch attached,
which makes executor skip initializing hash tables when doing explain only.
Best, Alex
Attachments:
[text/x-patch] dont-build-agg-hashtable-on-explain-only-v1.diff (651B, ../../36823f65-050d-ae24-aa4d-a37726998240@imap.cc/2-dont-build-agg-hashtable-on-explain-only-v1.diff)
download | inline diff:
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index d87677d659..196fe09c52 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3665,7 +3665,11 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
&aggstate->hash_ngroups_limit,
&aggstate->hash_planned_partitions);
find_hash_columns(aggstate);
- build_hash_tables(aggstate);
+
+ /* Skip massive memory allocation when running only explain */
+ if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY))
+ build_hash_tables(aggstate);
+
aggstate->table_filled = false;
/* Initialize this to 1, meaning nothing spilled, yet */
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: don't allocate HashAgg hash tables when running explain only
@ 2020-11-18 10:40 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Alexey Bashtanov <bashtanov@imap.cc>
0 siblings, 1 reply; 6+ messages in thread
From: Heikki Linnakangas @ 2020-11-18 10:40 UTC (permalink / raw)
To: Alexey Bashtanov <bashtanov@imap.cc>; pgsql-hackers
On 13/11/2020 18:10, Alexey Bashtanov wrote:
> Hi,
>
> I got somewhat scared when my explain took a few seconds to complete and
> used a few gigs of RAM.
> To reproduce try the following:
>
> discard temp;
> create temp table a as select to_timestamp(generate_series(1, 7000)) i;
> analyze a;
> set work_mem to '3GB';
> explain select distinct a1.i - a2.i from a a1, a a2;
>
> I would appreciate if someone could have a look at the patch attached,
> which makes executor skip initializing hash tables when doing explain only.
Makes sense. Committed, thanks for the patch!
- Heikki
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: don't allocate HashAgg hash tables when running explain only
@ 2020-11-19 05:20 Greg Stark <stark@mit.edu>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 6+ messages in thread
From: Greg Stark @ 2020-11-19 05:20 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Alexey Bashtanov <bashtanov@imap.cc>; pgsql-hackers
On Wed, 18 Nov 2020 at 05:40, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 13/11/2020 18:10, Alexey Bashtanov wrote:
>> > I would appreciate if someone could have a look at the patch attached,
> > which makes executor skip initializing hash tables when doing explain only.
>
> Makes sense. Committed, thanks for the patch!
Egads. That seems like a backpatchable bug fix to me. Have we been
doing this all along?!
--
greg
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: don't allocate HashAgg hash tables when running explain only
@ 2020-11-19 06:47 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Greg Stark <stark@mit.edu>
0 siblings, 1 reply; 6+ messages in thread
From: Heikki Linnakangas @ 2020-11-19 06:47 UTC (permalink / raw)
To: Greg Stark <stark@mit.edu>; +Cc: Alexey Bashtanov <bashtanov@imap.cc>; pgsql-hackers
On 19/11/2020 07:20, Greg Stark wrote:
> On Wed, 18 Nov 2020 at 05:40, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 13/11/2020 18:10, Alexey Bashtanov wrote:
>>>> I would appreciate if someone could have a look at the patch attached,
>>> which makes executor skip initializing hash tables when doing explain only.
>>
>> Makes sense. Committed, thanks for the patch!
>
> Egads. That seems like a backpatchable bug fix to me. Have we been
> doing this all along?!
Yeah, I believe it's always been like that. Yeah, arguably it should be
backpatched. I felt conservative and didn't backpatch, but feel free to
do it if you think it should be.
- Heikki
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: don't allocate HashAgg hash tables when running explain only
@ 2020-11-20 06:31 Michael Paquier <michael@paquier.xyz>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2020-11-20 06:31 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Greg Stark <stark@mit.edu>; Alexey Bashtanov <bashtanov@imap.cc>; pgsql-hackers
On Thu, Nov 19, 2020 at 08:47:51AM +0200, Heikki Linnakangas wrote:
> Yeah, I believe it's always been like that. Yeah, arguably it should be
> backpatched. I felt conservative and didn't backpatch, but feel free to do
> it if you think it should be.
+1 for a backpatch. The difference in runtime for EXPLAIN in this case
is quite something.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20201120063122.GE8506@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: don't allocate HashAgg hash tables when running explain only
@ 2020-11-20 13:30 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Heikki Linnakangas @ 2020-11-20 13:30 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Greg Stark <stark@mit.edu>; Alexey Bashtanov <bashtanov@imap.cc>; pgsql-hackers
On 20/11/2020 08:31, Michael Paquier wrote:
> On Thu, Nov 19, 2020 at 08:47:51AM +0200, Heikki Linnakangas wrote:
>> Yeah, I believe it's always been like that. Yeah, arguably it should be
>> backpatched. I felt conservative and didn't backpatch, but feel free to do
>> it if you think it should be.
>
> +1 for a backpatch. The difference in runtime for EXPLAIN in this case
> is quite something.
That's two votes for backpatching. Ok, I'll go do it.
- Heikki
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2020-11-20 13:30 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 16:10 don't allocate HashAgg hash tables when running explain only Alexey Bashtanov <bashtanov@imap.cc>
2020-11-18 10:40 ` Heikki Linnakangas <hlinnaka@iki.fi>
2020-11-19 05:20 ` Greg Stark <stark@mit.edu>
2020-11-19 06:47 ` Heikki Linnakangas <hlinnaka@iki.fi>
2020-11-20 06:31 ` Michael Paquier <michael@paquier.xyz>
2020-11-20 13:30 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox