mirror of
https://github.com/postgres/postgres.git
synced 2026-10-28 18:24:43 +08:00
Reintroduce hash_mem cap on buckets in BuildTupleHashTable
8f29467c5 removed the hash_mem cap from the nbuckets calculation in
BuildTupleHashTable() and put the trust into the planner to provide that
function with a sane value, which it would only cap at 2^32. There are
various places in the planner which cannot provide good estimates here
and it's been demonstrated that it's not difficult to craft a query
which requests 2^32 buckets.
Here we reintroduce the sanity checks so we don't end up making the hash
table's initial size to something that would exceed hash_mem. This may
cause the hash table to have to grow several times before it fills, but
that seems much better than having the executor OOM due to the request
being too large. Users are also used to work_mem and
hash_mem_multiplier having an effect on query performance, so this
shouldn't be too surprising. It also restores the limits that were
imposed pre-8f29467c5, which were tried and tested.
Bug: #19759
Reported-by: Alperen Keles <alpkeles99@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: shihao zhong <zhong950419@gmail.com>
Discussion: https://postgr.es/m/19759-acd3f33402e54f36@postgresql.org
Backpatch-through: 19
This commit is contained in:
1 parent
b936425002
commit
34d52e6d3d
1 file changed
+27
-8
@@ -203,18 +203,37 @@ BuildTupleHashTable(PlanState *parent,
|
||||
|
||||
/*
|
||||
* tuplehash_create requires a uint32 element count, so we had better
|
||||
* clamp the given nelements to fit in that. As long as we have to do
|
||||
* that, we might as well protect against completely insane input like
|
||||
* zero or NaN. But it is not our job here to enforce issues like staying
|
||||
* within hash_mem: the caller should have done that, and we don't have
|
||||
* enough info to second-guess.
|
||||
* clamp the given nelements to fit in that. We'd also better do
|
||||
* something sane when faced with a zero or NaN estimate. There are
|
||||
* various reasons why the planner might not be able to provide a good
|
||||
* estimate here; for example, recursive CTEs don't base their estimate
|
||||
* on anything reliable and can yield some rather large estimates. Here
|
||||
* we cap nbuckets to hash_mem to help prevent insane memory allocations.
|
||||
* The hash table can still grow when the bucket count is insufficient.
|
||||
* That will make the query a little slower, but that's better than
|
||||
* outright failing due to OOM if we were to request an insane bucket
|
||||
* count. Queries being slightly slower with smaller work_mem values is
|
||||
* also aligned to user expectations. We purposefully don't include the
|
||||
* tuple width in this calculation, as that would make the cap depend on
|
||||
* planner estimates for the tuple width, which could also be unreliable.
|
||||
*/
|
||||
if (isnan(nelements) || nelements <= 0)
|
||||
nbuckets = 1;
|
||||
else if (nelements >= PG_UINT32_MAX)
|
||||
nbuckets = PG_UINT32_MAX;
|
||||
else
|
||||
nbuckets = (uint32) nelements;
|
||||
{
|
||||
size_t entrysize;
|
||||
size_t hash_mem_limit;
|
||||
|
||||
if (nelements >= PG_UINT32_MAX)
|
||||
nbuckets = PG_UINT32_MAX;
|
||||
else
|
||||
nbuckets = (uint32) nelements;
|
||||
|
||||
entrysize = sizeof(TupleHashEntryData) + additionalsize;
|
||||
hash_mem_limit = get_hash_memory_limit() / entrysize;
|
||||
if (nbuckets > hash_mem_limit)
|
||||
nbuckets = hash_mem_limit;
|
||||
}
|
||||
|
||||
/* tuplescxt must be separate, else ResetTupleHashTable breaks things */
|
||||
Assert(metacxt != tuplescxt);
|
||||
|
||||
Reference in new issue
Block a user