On Sun, Dec 01, 2019 at 01:27:04PM -0500, Tom Lane wrote:
Tomas Vondra <tomas.von...@2ndquadrant.com> writes:
It seems most of this comesfrom find_mergeclauses_for_outer_pathkeys()
which builds matched_restrictinfos and then just leaves it allocated.
After pfreeing this (see attached patch), the memory usage gets way down
and the query completes.
Interesting. The memory leak was probably much less bad before
commit 1cff1b95a, since in the old List implementation this code
would have leaked only a list header. It makes sense to me to
add the list_free.
I forgot to mention I tried on older releases, up to 9.5 (I suspected it
might be related to parallel queries), and I get OOM crashes there too.
I can't say if the memory is leaking slower/faster, though.
I tried fixing 9.5 - a simple pfree(matched_restrictinfos) triggers some
sort of list_concat error for me, seemed a bit weird TBH.
Alternatively, it'd be possible to get rid of the separate List
altogether, and just add the rinfo's to "mergeclauses" immediately.
The functionality of the separate list could be replaced by a
bool variable remembering whether we found any matches in this
pass through the loop. I think the code would be a little less
clear that way, but this report makes it clear that it's a
performance bottleneck, so maybe we should just change it.
Yes, that might be an option. And it works even on 9.5 for me (per the
attached patch). I don't think it's much less clear compared to just
doing an explicit free at the end.
It does fix cases with up to join_collapse_limit = 10, but with 11 it
still OOM-crashes. That definitely depends on available memory, of
course.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/optimizer/path/pathkeys.c
b/src/backend/optimizer/path/pathkeys.c
index 928bbae8bd..a1d3f1dd3c 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -980,8 +980,8 @@ find_mergeclauses_for_outer_pathkeys(PlannerInfo *root,
{
PathKey *pathkey = (PathKey *) lfirst(i);
EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
- List *matched_restrictinfos = NIL;
ListCell *j;
+ bool found = false;
/*----------
* A mergejoin clause matches a pathkey if it has the same EC.
@@ -1027,8 +1027,12 @@ find_mergeclauses_for_outer_pathkeys(PlannerInfo *root,
clause_ec = rinfo->outer_is_left ?
rinfo->left_ec : rinfo->right_ec;
- if (clause_ec == pathkey_ec)
- matched_restrictinfos =
lappend(matched_restrictinfos, rinfo);
+
+ if (clause_ec != pathkey_ec)
+ continue;
+
+ mergeclauses = lappend(mergeclauses, rinfo);
+ found = true;
}
/*
@@ -1036,14 +1040,8 @@ find_mergeclauses_for_outer_pathkeys(PlannerInfo *root,
* sort-key positions in the pathkeys are useless. (But we can
still
* mergejoin if we found at least one mergeclause.)
*/
- if (matched_restrictinfos == NIL)
+ if (!found)
break;
-
- /*
- * If we did find usable mergeclause(s) for this sort-key
position,
- * add them to result list.
- */
- mergeclauses = list_concat(mergeclauses, matched_restrictinfos);
}
return mergeclauses;
