Le lun. 29 nov. 2021 à 19:40, Tom Lane <t...@sss.pgh.pa.us> a écrit :
> Justin Pryzby <pry...@telsasoft.com> writes:
> > I reproduced the issue like this.
>
> > psql postgres -c 'CREATE ROLE two WITH login superuser'
> > psql postgres two -c "SELECT lo_import('/dev/null') FROM
> generate_series(1,22111)" >/dev/null
> > psql postgres -c 'SET client_min_messages=debug; SET
> log_statement_stats=on;' -c 'begin; REASSIGN OWNED BY two TO pryzbyj;
> rollback;'
>
> Confirmed here, although I needed to use a lot more than 22K large objects
> to see a big leak.
>
>
So do I.
> I didn't find the root problem, but was able to avoid the issue by
> creating a
> > new mem context. I wonder if there are a bunch more issues like this.
>
> I poked into it with valgrind, and identified the major leak as being
> stuff that is allocated by ExecOpenIndices and not freed by
> ExecCloseIndices. The latter knows it's leaking:
>
> /*
> * XXX should free indexInfo array here too? Currently we assume
> that
> * such stuff will be cleaned up automatically in
> FreeExecutorState.
> */
>
> On the whole, I'd characterize this as DDL code using pieces of the
> executor without satisfying the executor's expectations as to environment
> --- specifically, that it'll be run in a memory context that doesn't
> outlive executor shutdown. Normally, any one DDL operation does a limited
> number of catalog updates so that small per-update leaks don't cost that
> much ... but REASSIGN OWNED creates a loop that can invoke ALTER OWNER
> many times.
>
> I think your idea of creating a short-lived context is about right.
> Another idea we could consider is to do that within CatalogTupleUpdate;
> but I'm not sure that the cost/benefit ratio would be good for most
> operations. Anyway I think ALTER OWNER has other leaks outside the
> index-update operations, so we'd still need to do this within
> REASSIGN OWNED's loop.
>
>
I've tried Justin's patch but it didn't help with my memory allocation
issue. FWIW, I attach the patch I used in v14.
DROP OWNED BY likely has similar issues.
>
>
Didn't try it, but it wouldn't be a surprise.
--
Guillaume.
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index d89a9fa2bf..68dad27afb 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -65,6 +65,7 @@
#include "storage/lmgr.h"
#include "utils/acl.h"
#include "utils/fmgroids.h"
+#include "utils/memutils.h"
#include "utils/syscache.h"
typedef enum
@@ -1549,6 +1550,10 @@ shdepReassignOwned(List *roleids, Oid newrole)
while ((tuple = systable_getnext(scan)) != NULL)
{
Form_pg_shdepend sdepForm = (Form_pg_shdepend) GETSTRUCT(tuple);
+ MemoryContext cxt, oldcxt;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext, "shdepReassignOwned cxt", ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
/*
* We only operate on shared objects and objects in the current
@@ -1656,6 +1661,9 @@ shdepReassignOwned(List *roleids, Oid newrole)
}
/* Make sure the next iteration will see my changes */
CommandCounterIncrement();
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
}
systable_endscan(scan);
