Date: Sun, 28 Jan 2018 17:02:13 -0800
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	"Todd A. Cook" <tcook@blackducksoftware.com>,
	Thomas Munro <thomas.munro@enterprisedb.com>,
	PostgreSQL Bugs <pgsql-bugs@postgresql.org>
Subject: Re: BUG #14932: SELECT DISTINCT val FROM table gets stuck in an
 infinite loop
Message-ID: <20180129010213.rt3nu73ilff756f6@alap3.anarazel.de>
References: <9cd29b81-7ba1-91a2-f491-9014f4ac87e6@blackducksoftware.com>
 <2a76784c-06cb-46ec-35be-9895d144b839@2ndquadrant.com>
 <219b37b2-d740-d04a-203a-49ae422028f7@blackducksoftware.com>
 <c15378b3-d4aa-a5c0-5538-dd52904dcbc6@2ndquadrant.com>
 <1773c302-2fb1-749f-8ccc-62b23617c5ac@blackducksoftware.com>
 <25629.1512580464@sss.pgh.pa.us>
 <20171206171923.dg7phzfh7nb4lou3@alap3.anarazel.de>
 <7de4b2d6-b119-7d15-04d8-4c0fcb29cbc3@2ndquadrant.com>
 <20180126234537.dyu6jwnqz7snpq7n@alap3.anarazel.de>
 <14184.1517010515@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="erogdy2pp7ibjmmn"
Content-Disposition: inline
In-Reply-To: <14184.1517010515@sss.pgh.pa.us>
Precedence: bulk


--erogdy2pp7ibjmmn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On 2018-01-26 18:48:35 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-12-10 23:09:42 +0100, Tomas Vondra wrote:
> >> FWIW I do agree the data sets shared in this thread are pretty extreme
> >> and it doesn't make much sense to slow the regular cases. I'll be
> >> perfectly happy if we stop the OOM, making those cases fast is a bonus.
> 
> > Yea, agreed on that. I'm kinda inclined to go for stop-growing in 10,
> > and so something better in 11. And then later possibly backpatch if
> > we've grown some confidence?
> 
> +1.  I'd like to see some response to this included in 10.2, and time
> grows short for that.

Here are two patches that I think we want for 10.2, and the start of one
that I think we want for master.  0002 is needed because otherwise the
lack of extra growth leads to noticeably worse performance when filling
an underestimated a coordinator hash table from the workers - turns out
our hash combine (and most hash combines) let a lot of clustering
survive. By adding a final hashing round the bit perturbation is near
perfect.  The commit messages need to be polished a bit, but other than
that I think these are reasonable fixes. Plan to push by Monday evening
at the latest.

The third patch is a version of the random IV discussed in this
thread. I do think we want to add usage of the extended hash functions,
as prototyped by Tomas, as that actually helps to fix issues with actual
hash conflicts. But we additionally need a fallback path for types
without extended hashtables, and the random IV is a good idea
nonetheless.  There's no ABI difference in my patch, so I think this is
actually something we could backpatch. But I don't think it's urgent, so
I'm not planning to do that for 10.2.  The one thing that could confuse
people is that it can lead to output order changes from run to run - I
think that's actually good, nobody should rely on hashagg etc output
being stable, but it might be a bit much in a stable release?


In my tests this solves the worst performance issues in Todd's case,
Tomas's, Thomas's and still does ok performancwith with a TPC-H Q18
(which showcases the underestimated worker hashtable into leader
hashtable issue).

Greetings,

Andres Freund

--erogdy2pp7ibjmmn
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Prevent-growth-of-simplehash-tables-when-they-re-too.patch"