pool cleanup (was: Re: New post-log-transaction hook?)

[Greg Stein <gs...@lyra.org>]

On Wed, Sep 19, 2001 at 12:16:24PM -0700, Ryan Bloom wrote:
> On Wednesday 19 September 2001 11:37 am, William A. Rowe, Jr. wrote:
> > From: "Greg Stein" <gs...@lyra.org>
> > Sent: Wednesday, September 19, 2001 1:26 PM
>...
> > > The problem is cross-dependency between the cleanup actions. One can
> > > destroy something another cleanup needs. If you restrict their actions to
> > > "simple" things, then the cross-dependencies are (hopefully!) removed.
> >
> > Really?  No.  Cleanups are run as a LIFO stack.  Anything that existed when
> > something was added to the pool must exist when that something is removed
> > from the pool.

They are not strictly LIFO. You can remove a cleanup and insert a new one at
any time. Let's say that the cleanup list looked like:

    cleanups: A

and you add a new one to the "front":

    cleanups: B A

and now case 1, where A needs to rejigger its cleanup param a bit:

    cleanups: A' B

or case 2, where A simply removes its cleanup:

    cleanups: B


Case 2 actually happens quite often.

> > IMHO, we need to make subpool scrubbing an actual LIFO cleanup as well, so
> > that will also be true of subpools.

Subpools are entire entities. There shouldn't be any cross-dependencies
between those. Care in ordering isn't necessary.

> > Considering how we use pools for dynamic libraries and the rest, it's
> > absolutely vital that they are unspun from the pool in LIFO order of their
> > creation.

Ah. That is a good example... had forgotten about that one (I even ran into
this once). Using the symbology from above, what happens if B is using code
from a dso that installed A? What happens if A is unloaded and reloaded,
thus rearrange the queue as in case 1.


The point is: I've observed problems with cleanup ordering. And the DSO
thing is particular nasty. At one point, I was considering adjusting
Apache's pool structure to better order when the DSOs were unloaded. For
example, if we loaded DSOs into Pool X and then used subpool Y for all
module-related allocations (e.g. pconf == Y and X is internal).

> I agree with Bill.  Having reviewed the code quite deeply yesterday, pool
> cleanups follow a very clean rule, and registering a cleanup from within
> a cleanup will always work if done correctly.

Huh? If you register a cleanup within a cleanup, it *won't* be run. We grab
the list, then run it. Most types of changes to that list won't be seen;
particularly insertions at the front of it.

> If you have data in a pool
> when the cleanup is registered, it is gauranteed to be there when the cleanup
> is run.

The pool data might, but associated resources may not be. For example, the
apr_file_t structure might still be in the pool (simply cuz it can't be
freed), but the underlying file descriptor could be closed.

> Anything else is completely broken.

Depends on your interpretation :-)  I think the cleanup behavior is very
well defined, and has certain restrictions on the kinds of things that you
can do in there. And one of those restrictions is watching out for
dependending on something else in your pool. As a result, arbitrary work can
definitely be affected.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: pool cleanup

[Greg Stein <gs...@lyra.org>]

On Thu, Sep 20, 2001 at 07:02:58AM -0700, Ryan Bloom wrote:
> On Wednesday 19 September 2001 02:21 pm, Greg Stein wrote:
>...
> > They are not strictly LIFO. You can remove a cleanup and insert a new one
> > at any time. Let's say that the cleanup list looked like:
> >
> >     cleanups: A
> >
> > and you add a new one to the "front":
> >
> >     cleanups: B A
> >
> > and now case 1, where A needs to rejigger its cleanup param a bit:
> >
> >     cleanups: A' B
> >
> > or case 2, where A simply removes its cleanup:
> >
> >     cleanups: B
> >
> >
> > Case 2 actually happens quite often.
> 
> This is all true, but it is also orthogonal to this conversation.

Partly. The conversation moved into "what can you do in a cleanup". If you
want to look at the simple issue of registering cleanups... okay. But when
people were expecting to be able to do "anything" in a cleanup... that is
intrinsically incorrect.

> The question we are
> trying to answer here, is can you register a cleanup within a cleanup.

Aaron posted a patch, but it introduces too many function calls in the
processing. I posted one that is much more optimal, processing the cleanups
in batches. That would fix your issue.

> If we are in
> the middle of running the cleanups, and somebody actually calls cleanup_run 
> or cleanup_kill from within a cleanup, they are broken and it may not work.

My above case wasn't talking about doing those from within a cleanup (which
is definitely and always wrong). I was showing how the cleanups could be
reordered; therefore, how you cannot depend upon particular cross-cleanup
ordering dependencies. Thus, you are actually somewhat limited in what kinds
of things you can truly do in a cleanup.

> It also doesn't make any sense, because the reason to run a cleanup, is to perform
> some action sooner than you would have otherwise, but in this case, we are going
> to perform that action in a few seconds anyway.

I don't get this part. A cleanup is to do just that: clean up after yourself
when the pool goes away. It provides a point in time for specific types of
actions. I'm not sure how that gives you "sooner"; if anything, a cleanup is
for running things later.

> Since the two cases above require a programer to either remove or run a cleanup,
> they don't really make sense in the context of registering a cleanup within a cleanup.
> This means that is safe to register a cleanup within a cleanup, assuming the code
> is patched correctly.

Agreed. My point was addressing the "arbitrary work in a cleanup" meme that
was brought up.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: pool cleanup (was: Re: New post-log-transaction hook?)

[Ryan Bloom <rb...@covalent.net>]

On Wednesday 19 September 2001 02:21 pm, Greg Stein wrote:
> On Wed, Sep 19, 2001 at 12:16:24PM -0700, Ryan Bloom wrote:
> > On Wednesday 19 September 2001 11:37 am, William A. Rowe, Jr. wrote:
> > > From: "Greg Stein" <gs...@lyra.org>
> > > Sent: Wednesday, September 19, 2001 1:26 PM
> > > Really?  No.  Cleanups are run as a LIFO stack.  Anything that existed
> > > when something was added to the pool must exist when that something is
> > > removed from the pool.
>
> They are not strictly LIFO. You can remove a cleanup and insert a new one
> at any time. Let's say that the cleanup list looked like:
>
>     cleanups: A
>
> and you add a new one to the "front":
>
>     cleanups: B A
>
> and now case 1, where A needs to rejigger its cleanup param a bit:
>
>     cleanups: A' B
>
> or case 2, where A simply removes its cleanup:
>
>     cleanups: B
>
>
> Case 2 actually happens quite often.

This is all true, but it is also orthogonal to this conversation. The question we are
trying to answer here, is can you register a cleanup within a cleanup. If we are in
the middle of running the cleanups, and somebody actually calls cleanup_run 
or cleanup_kill from within a cleanup, they are broken and it may not work.
It also doesn't make any sense, because the reason to run a cleanup, is to perform
some action sooner than you would have otherwise, but in this case, we are going
to perform that action in a few seconds anyway.

Since the two cases above require a programer to either remove or run a cleanup,
they don't really make sense in the context of registering a cleanup within a cleanup.
This means that is safe to register a cleanup within a cleanup, assuming the code
is patched correctly.

Ryan
______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------