You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bill Stoddard <bi...@wstoddard.com> on 2002/03/04 23:51:29 UTC
apr_file_open() and caching file descriptors
I am trying to make mod_mem_cache cache file descriptors. When opening a file to be shared
across threads, we must call apr_file_open() with the APR_XTHREAD option (non-cached files
are not and should not be opened with this option).
So, when I am about to cache a file I am serving, I need to use the
apr_file_open(APR_XTHREAD) call to re-open the file to be put in the cache. What pool to
pass into the apr_file_open call? I could pass in r->pool, but then the apr_file_t would
be cleaned up at the end of the request, which is not right. I could pass in some long
lived pool but then the pool storage would not be cleaned up when the file is garbage
collected (because it has expired, changed, whatever). It is unreasonable to give each
open file its own pool (unless we can make that pool precisely the size of the apr_file_t,
which I am assuming cannot be done in a reasonable fashion...). A conundrum...
I would like to be able to call apr_file_open with an option to suppress registering the
cleanup, something like this...
apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
Then call apr_os_file_get() to retrieve the file descriptor which is the thing I will put
in the cache object. When we serve a file out of the cache object, we can call
apr_os_file_put(cache->fd, r->pool); to create the apr_file_t that we then place in a file
bucket.
Thoughts?
Bill
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
> At 05:29 PM 3/4/2002, you wrote:
> >On Mon, 4 Mar 2002, Bill Stoddard wrote:
> >
> > > Rather than an option to not register a cleanup, perhaps a function to
> > > kill the cleanup would be more generally useful.
> > >
> > > apr_file_kill_cleanups(apr_file_t *file);
> >
> >You still have a problem with the apr_file_t disappearing when r->pool
> >goes away, meaning you'd still need the apr_os_file_get/put thing, which
> >just doesn't seem like a good idea to me. There has got to be a good way
> >to do this... I'll keep thinking on it and get back to you asap.
>
> Caching introduces tons o' headaches.
>
> You won't like it, but you need a cross process cache pool derived from
> process->pool.
Nahhh, don't need cross process squat. If you are interested in cross process caching, use
mod_file_cache and preload the cache before the fork. I am completely not interested in
generalizing mod_mem_cache to handle cross process caching at this point.
> Individual cache entries should each gain a subpool of
> their own ... if you do it right, the 'remove from cache' function is a cleanup
> of that subpool.
And perhaps this is a good direction to look in. I originally discarded the thought until
Cliff pointed out the APR_XTHREAD breakage if we do not maintain an apr_file_t. The
subpool must be exactly the size needed to hold the apr_file_t (and perhaps slightly
larger). Today, the size of the subpool is way to large...
Bill
Re: apr_file_open() and caching file descriptors
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 05:29 PM 3/4/2002, you wrote:
>On Mon, 4 Mar 2002, Bill Stoddard wrote:
>
> > Rather than an option to not register a cleanup, perhaps a function to
> > kill the cleanup would be more generally useful.
> >
> > apr_file_kill_cleanups(apr_file_t *file);
>
>You still have a problem with the apr_file_t disappearing when r->pool
>goes away, meaning you'd still need the apr_os_file_get/put thing, which
>just doesn't seem like a good idea to me. There has got to be a good way
>to do this... I'll keep thinking on it and get back to you asap.
Caching introduces tons o' headaches.
You won't like it, but you need a cross process cache pool derived from
process->pool. Individual cache entries should each gain a subpool of
their own ... if you do it right, the 'remove from cache' function is a cleanup
of that subpool. So you create a cache subpool for an entry, initialize it,
put it in the cache list, and when they go to destroy that 'entry' (the pool
itself) the file and memory is cleaned up, and the entry removed from the
cache list.
That's the most elegant design I can picture. Performance wise??? well...
I don't have a good idea of how that would perform.
The only obvious alternative is to 'reuse' apr_file_t's with apr_file_os_put
or apr_file_dup or something like that. It's a legit option worth considering.
Bill
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
Will submit a patch later on this evening...
Bill
----- Original Message -----
From: "Bill Stoddard" <bi...@wstoddard.com>
To: "Cliff Woolley" <jw...@virginia.edu>
Cc: <de...@httpd.apache.org>; "APR Development List" <de...@apr.apache.org>
Sent: Tuesday, March 05, 2002 5:24 PM
Subject: Re: apr_file_open() and caching file descriptors
> Haven't had much time to think about this today, but I did discover that the XTHREAD
> support in win32 apr_file io is seriously broken. apr_file_open(APR_XTHREAD) on Windows
> should -not- be creating the overlapped structure and the io completion event. If an
open
> file is being shared across thread, each thread should have it's own instance of
> apr_file_t with its own instance of the overlapped structure and the completion event.
As
> it is now, if two threads both try to read from a file opened for overlapped i/o at the
> same time, thread 1 might get the io completion notification for the io issued by thread
2
> or visa-versa. Not good.
>
> Bill
>
> > On Mon, 4 Mar 2002, Bill Stoddard wrote:
> >
> > > > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
> >
> > I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
> > whatever it would be called), but I do have a problem with using
> > apr_os_file_get/apr_os_file_put for this. There has got to be a better
> > way. (For one thing, your APR_XTHREAD flag is meaningless if you do
> > that.)
> >
> > > Rather than an option to not register a cleanup, perhaps a function to
> > > kill the cleanup would be more generally useful.
> > >
> > > apr_file_kill_cleanups(apr_file_t *file);
> >
> > You still have a problem with the apr_file_t disappearing when r->pool
> > goes away, meaning you'd still need the apr_os_file_get/put thing, which
> > just doesn't seem like a good idea to me. There has got to be a good way
> > to do this... I'll keep thinking on it and get back to you asap.
> >
> > --Cliff
> >
> > --------------------------------------------------------------
> > Cliff Woolley
> > cliffwoolley@yahoo.com
> > Charlottesville, VA
> >
> >
>
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
Will submit a patch later on this evening...
Bill
----- Original Message -----
From: "Bill Stoddard" <bi...@wstoddard.com>
To: "Cliff Woolley" <jw...@virginia.edu>
Cc: <de...@httpd.apache.org>; "APR Development List" <de...@apr.apache.org>
Sent: Tuesday, March 05, 2002 5:24 PM
Subject: Re: apr_file_open() and caching file descriptors
> Haven't had much time to think about this today, but I did discover that the XTHREAD
> support in win32 apr_file io is seriously broken. apr_file_open(APR_XTHREAD) on Windows
> should -not- be creating the overlapped structure and the io completion event. If an
open
> file is being shared across thread, each thread should have it's own instance of
> apr_file_t with its own instance of the overlapped structure and the completion event.
As
> it is now, if two threads both try to read from a file opened for overlapped i/o at the
> same time, thread 1 might get the io completion notification for the io issued by thread
2
> or visa-versa. Not good.
>
> Bill
>
> > On Mon, 4 Mar 2002, Bill Stoddard wrote:
> >
> > > > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
> >
> > I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
> > whatever it would be called), but I do have a problem with using
> > apr_os_file_get/apr_os_file_put for this. There has got to be a better
> > way. (For one thing, your APR_XTHREAD flag is meaningless if you do
> > that.)
> >
> > > Rather than an option to not register a cleanup, perhaps a function to
> > > kill the cleanup would be more generally useful.
> > >
> > > apr_file_kill_cleanups(apr_file_t *file);
> >
> > You still have a problem with the apr_file_t disappearing when r->pool
> > goes away, meaning you'd still need the apr_os_file_get/put thing, which
> > just doesn't seem like a good idea to me. There has got to be a good way
> > to do this... I'll keep thinking on it and get back to you asap.
> >
> > --Cliff
> >
> > --------------------------------------------------------------
> > Cliff Woolley
> > cliffwoolley@yahoo.com
> > Charlottesville, VA
> >
> >
>
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
Haven't had much time to think about this today, but I did discover that the XTHREAD
support in win32 apr_file io is seriously broken. apr_file_open(APR_XTHREAD) on Windows
should -not- be creating the overlapped structure and the io completion event. If an open
file is being shared across thread, each thread should have it's own instance of
apr_file_t with its own instance of the overlapped structure and the completion event. As
it is now, if two threads both try to read from a file opened for overlapped i/o at the
same time, thread 1 might get the io completion notification for the io issued by thread 2
or visa-versa. Not good.
Bill
> On Mon, 4 Mar 2002, Bill Stoddard wrote:
>
> > > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
>
> I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
> whatever it would be called), but I do have a problem with using
> apr_os_file_get/apr_os_file_put for this. There has got to be a better
> way. (For one thing, your APR_XTHREAD flag is meaningless if you do
> that.)
>
> > Rather than an option to not register a cleanup, perhaps a function to
> > kill the cleanup would be more generally useful.
> >
> > apr_file_kill_cleanups(apr_file_t *file);
>
> You still have a problem with the apr_file_t disappearing when r->pool
> goes away, meaning you'd still need the apr_os_file_get/put thing, which
> just doesn't seem like a good idea to me. There has got to be a good way
> to do this... I'll keep thinking on it and get back to you asap.
>
> --Cliff
>
> --------------------------------------------------------------
> Cliff Woolley
> cliffwoolley@yahoo.com
> Charlottesville, VA
>
>
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
Haven't had much time to think about this today, but I did discover that the XTHREAD
support in win32 apr_file io is seriously broken. apr_file_open(APR_XTHREAD) on Windows
should -not- be creating the overlapped structure and the io completion event. If an open
file is being shared across thread, each thread should have it's own instance of
apr_file_t with its own instance of the overlapped structure and the completion event. As
it is now, if two threads both try to read from a file opened for overlapped i/o at the
same time, thread 1 might get the io completion notification for the io issued by thread 2
or visa-versa. Not good.
Bill
> On Mon, 4 Mar 2002, Bill Stoddard wrote:
>
> > > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
>
> I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
> whatever it would be called), but I do have a problem with using
> apr_os_file_get/apr_os_file_put for this. There has got to be a better
> way. (For one thing, your APR_XTHREAD flag is meaningless if you do
> that.)
>
> > Rather than an option to not register a cleanup, perhaps a function to
> > kill the cleanup would be more generally useful.
> >
> > apr_file_kill_cleanups(apr_file_t *file);
>
> You still have a problem with the apr_file_t disappearing when r->pool
> goes away, meaning you'd still need the apr_os_file_get/put thing, which
> just doesn't seem like a good idea to me. There has got to be a good way
> to do this... I'll keep thinking on it and get back to you asap.
>
> --Cliff
>
> --------------------------------------------------------------
> Cliff Woolley
> cliffwoolley@yahoo.com
> Charlottesville, VA
>
>
Re: apr_file_open() and caching file descriptors
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 4 Mar 2002, Bill Stoddard wrote:
> > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
whatever it would be called), but I do have a problem with using
apr_os_file_get/apr_os_file_put for this. There has got to be a better
way. (For one thing, your APR_XTHREAD flag is meaningless if you do
that.)
> Rather than an option to not register a cleanup, perhaps a function to
> kill the cleanup would be more generally useful.
>
> apr_file_kill_cleanups(apr_file_t *file);
You still have a problem with the apr_file_t disappearing when r->pool
goes away, meaning you'd still need the apr_os_file_get/put thing, which
just doesn't seem like a good idea to me. There has got to be a good way
to do this... I'll keep thinking on it and get back to you asap.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: apr_file_open() and caching file descriptors
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 4 Mar 2002, Bill Stoddard wrote:
> > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
I don't conceptually have a problem with the APR_NO_CLEANUP flag (or
whatever it would be called), but I do have a problem with using
apr_os_file_get/apr_os_file_put for this. There has got to be a better
way. (For one thing, your APR_XTHREAD flag is meaningless if you do
that.)
> Rather than an option to not register a cleanup, perhaps a function to
> kill the cleanup would be more generally useful.
>
> apr_file_kill_cleanups(apr_file_t *file);
You still have a problem with the apr_file_t disappearing when r->pool
goes away, meaning you'd still need the apr_os_file_get/put thing, which
just doesn't seem like a good idea to me. There has got to be a good way
to do this... I'll keep thinking on it and get back to you asap.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
> I am trying to make mod_mem_cache cache file descriptors. When opening a file to be
shared
> across threads, we must call apr_file_open() with the APR_XTHREAD option (non-cached
files
> are not and should not be opened with this option).
>
> So, when I am about to cache a file I am serving, I need to use the
> apr_file_open(APR_XTHREAD) call to re-open the file to be put in the cache. What pool to
> pass into the apr_file_open call? I could pass in r->pool, but then the apr_file_t would
> be cleaned up at the end of the request, which is not right. I could pass in some long
> lived pool but then the pool storage would not be cleaned up when the file is garbage
> collected (because it has expired, changed, whatever). It is unreasonable to give each
> open file its own pool (unless we can make that pool precisely the size of the
apr_file_t,
> which I am assuming cannot be done in a reasonable fashion...). A conundrum...
>
> I would like to be able to call apr_file_open with an option to suppress registering the
> cleanup, something like this...
>
> apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
Rather than an option to not register a cleanup, perhaps a function to kill the cleanup
would be more generally useful.
apr_file_kill_cleanups(apr_file_t *file);
>
> Then call apr_os_file_get() to retrieve the file descriptor which is the thing I will
put
> in the cache object. When we serve a file out of the cache object, we can call
> apr_os_file_put(cache->fd, r->pool); to create the apr_file_t that we then place in a
file
> bucket.
>
> Thoughts?
>
> Bill
>
Re: apr_file_open() and caching file descriptors
Posted by Bill Stoddard <bi...@wstoddard.com>.
> I am trying to make mod_mem_cache cache file descriptors. When opening a file to be
shared
> across threads, we must call apr_file_open() with the APR_XTHREAD option (non-cached
files
> are not and should not be opened with this option).
>
> So, when I am about to cache a file I am serving, I need to use the
> apr_file_open(APR_XTHREAD) call to re-open the file to be put in the cache. What pool to
> pass into the apr_file_open call? I could pass in r->pool, but then the apr_file_t would
> be cleaned up at the end of the request, which is not right. I could pass in some long
> lived pool but then the pool storage would not be cleaned up when the file is garbage
> collected (because it has expired, changed, whatever). It is unreasonable to give each
> open file its own pool (unless we can make that pool precisely the size of the
apr_file_t,
> which I am assuming cannot be done in a reasonable fashion...). A conundrum...
>
> I would like to be able to call apr_file_open with an option to suppress registering the
> cleanup, something like this...
>
> apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool);
Rather than an option to not register a cleanup, perhaps a function to kill the cleanup
would be more generally useful.
apr_file_kill_cleanups(apr_file_t *file);
>
> Then call apr_os_file_get() to retrieve the file descriptor which is the thing I will
put
> in the cache object. When we serve a file out of the cache object, we can call
> apr_os_file_put(cache->fd, r->pool); to create the apr_file_t that we then place in a
file
> bucket.
>
> Thoughts?
>
> Bill
>