Re: DISCARD ALL ; stored procedures

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Fri, Jan 7, 2011 at 1:29 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > #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.
>
> I'm guessing that just resetting the memory context is going to result
> in things breaking all over the place - the PL might have dangling
> pointers into the context.

After looking through the code more, we actually already say "use this
context for stuff you allocate in fn_extra", but it doesn't look like
the PLs are respecting or using that. We do have a function which
resets fn_extra already (fmgr_finfo_copy) but I'm not sure under what
conditions it's used and I'm not sure why it doesn't leak memory by
doing that.

If we can figure out the list of functions that have been called, get at
all of their fn_extra pointers to set them to NULL, and nuke the context
that they're created in, that should work. The PLs in core appear to be
good about using fn_extra and resetting it should be sufficient to force
a recompile of the stored procedures. It also looks like they shouldn't
have any issue surviving that reset.

> And it might have other resources that we
> don't know about. Thus I think we need:

This is certainly a concern and would be a reason to offer a seperate
function for the PLs to use, but I'm not sure we need to jump there
right away. I'd like to see if the core/contrib PLs can all handle the
above approach and then see if third-party PLs complain.

> > #2. Add another entry point to the PLs in pg_pltemplate.h which is a
> >    function to be called on DISCARD.
>
> ...except I think that the principal thing you need to modify is
> pl_language, rather than pl_pltemplate.

Right, sorry.

> If we go this route, then (1) it can't be back-patched, obviously, and
> (2) we need to think a little bit harder about what we're asking to
> have discarded, because I think it's going to be a lot more than just
> cached plans.

I'm not ready to give up quite yet, but I agree that we might end up
there.

Thanks,

Stephen