Re: DISCARD ALL ; stored procedures

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Merlin Moncure <mmoncure(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: DISCARD ALL ; stored procedures
Date: 2011-01-07 18:29:52
Message-ID: 20110107182952.GQ4933@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> > > Making it part of DISCARD PLANS; and back-patching it to 8.3 where
> > > DISCARD was introduced would be awesome for me. :)
> >
> > I'd need to look at this more closely before committing anything, but
> > at first blush I think that's reasonable. Have a patch?
>
> To be honest, I was fully expecting a response of "that's hard to do."

Soo, yeah, I found the "this is hard" part. Basically, the plan
cacheing, etc, is all handled by the stored procedures themselves, and
we havn't got any way to tell a PL "destroy all your plans." We might
be able to hack up fmgr to throw away all references to functions, but
that wouldn't release the memory they use up, making 'discard plans;'
leak like a sieve.

What's worse is that most PLs actually *also* allocate a bunch of stuff
in TopMemoryContext, meaning even if we figured out what contexts are
used by the PLs, we still couldn't nuke them. Personally, I'm not a fan
of the PLs doing that (perhaps there's some performance reason? dunno).

A few approaches come to mind for dealing with this-

#1. Add a new 'Top-but-removed-on-DISCARD' context and modify the PLs to
use that instead of TopMemoryContext and require any other contexts
they create to be children of it.

#2. Add another entry point to the PLs in pg_pltemplate.h which is a
function to be called on DISCARD.

#3. Add a way for PLs to request a context similar to #1, but would be
on a per-PL basis. This would involve adding a new function to
mmgr/, or adding a new parameter for creating a context.

I like the idea of having a context-per-PL, but I don't know that I can
justify the complications necessary to make it happen. Given that
they're all already dumping things in TopMemoryContext, at least moving
them out of *that* would improve the situation, so my inclination is to
do #1. Of course, user-added PLs wouldn't get this benefit and would
still leak memory after a DISCARD until they're updated.

After the above is figured out, we should be able to go through and
make fmgr get cleaned up after a DISCARD.

Thoughts?

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2011-01-07 18:46:00 Re: system views for walsender activity
Previous Message Jim Nasby 2011-01-07 18:28:54 Re: crash-safe visibility map, take three