Re: OOM on EXPLAIN with lots of nodes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Alexey Bashtanov <bashtanov(at)imap(dot)cc>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: OOM on EXPLAIN with lots of nodes
Date: 2015-01-13 18:10:19
Message-ID: 23580.1421172619@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> On 01/13/2015 07:24 PM, Tom Lane wrote:
>> In hindsight, that's a bad API and we should change it to something like
>> ExplainState *es = NewExplainState();
>> so that the sizeof the struct isn't embedded in extension code. But we
>> definitely can't do that in back branches.

> Actually, it would make sense to do exactly that, to break any
> extensions that are doing the unsafe thing in an obvious way. The
> downside would be that an extension using the new API would then not
> work on an old server.

I guess that's a possibility ...

> We could repurpose one of the existing fields in ExplainState to point
> to another struct that contains more fields. Something like this:
> ...
> That's pretty ugly, but it would work even if there are ExplainState
> structs embeded in extensions. As long as they don't try to look at the
> grouping_stack field; I think that's fairly safe assumption.

Yeah, I was thinking the same thing, but it's *mighty* ugly and would also
create a back-patch hazard, since presumably we'd not do that in HEAD.

> But do we really need to backpatch any of this?

Alexey's example consumes only a couple hundred MB in 9.2, vs about 7GB
peak in 9.3 and up. That seems like a pretty nasty regression.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2015-01-13 18:11:31 Re: Check that streaming replica received all data after master shutdown
Previous Message Heikki Linnakangas 2015-01-13 17:52:50 Re: [PATCH] explain sortorder