Re: Performance degradation in TPC-H Q18

From: Andres Freund <andres(at)anarazel(dot)de>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Performance degradation in TPC-H Q18
Date: 2017-02-28 17:46:13
Message-ID: 20170228174613.s5zcpvuxwjh3mw4h@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2017-02-26 19:30:32 +0530, Robert Haas wrote:
> On Wed, Feb 22, 2017 at 11:23 AM, Kuntal Ghosh
> <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
> > While conducting the experiments for parallelism, Rafia came across a
> > hang in Q18 when plan uses partial and finalize hash aggregate. This
> > could be seen on both scale factors - 20 and 300, on setting work_mem
> > high enough so that the query uses hash aggregate. It seems that
> > commit b81b5a96f424531b97cdd1dba97d9d1b9c9d372e does not solve the
> > issue completely.
>
> Andres, any thoughts? Isn't the same issue that we were discussing
> https://www.postgresql.org/message-id/CA+TgmoYNO8qouPVO=1Q2aXzuxe942d_T5bcvZd9iKOC9tb3uLg@mail.gmail.com
> over a month ago?

Yes, I presume that it is.

> To me, it seems like a big problem that we have large, unfixed
> performance regressions with simplehash four months after it went in.

Yea, I agree. I'm fairly sure that the patch I posted in that thread
actually fixes the issue (and would also have made already applied hash
patch of yours a small-ish optimization). I held back back because I
disliked the idea of magic constants, and I couldn't figure out a way to
properly "derive" them - but I'm inclined to simply live with the magic constsnts.

> I hate to suggest ripping the whole thing out, and it seems like
> overkill

Yea, I don't think we're there yet. Let me push the patch that resizes
adaptively, and we can see how things are going from there.

> , but it's pretty clear to me that the current state of things
> is unacceptable, and that we're going to have a lot of unhappy users
> if we ship it the way that it is right now.

Yea, if we can't improve upon the current state, we'd need to revert.

> I want to point out that
> the kinds of problems we're hitting here are exactly what I told you I
> was worried about before it went in - that the average-case
> performance would be better but that there would be
> all-too-easy-to-hit cases where things got much worse because the
> whole thing degenerated into a linear search. Not only did that
> happen, but there seem to be multiple ways of producing it without
> half trying, of which b81b5a96f424531b97cdd1dba97d9d1b9c9d372e fixed
> only one.

Note that that one is also fixed with what I'm proposing (but it's a
worthwhile small-ish improvement nonetheless).

> Something that's 5-10% faster in common cases but 2x or 10x
> slower when things go wrong is not an improvement.

The hash-table part is more like 3x faster - but as so often when making
things faster, the next bottleneck is just around the corner...

- Andres

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2017-02-28 17:49:05 Re: Allow pg_dumpall to work without pg_authid
Previous Message Andreas Karlsson 2017-02-28 17:21:39 Re: REINDEX CONCURRENTLY 2.0