Ruediger Pluem wrote:

On 12/27/2008 07:27 PM, André Malo wrote:
* Ruediger Pluem wrote:

On 12/27/2008 04:07 PM, André Malo wrote:
* Ruediger Pluem wrote:
On 12/26/2008 10:41 PM, [email protected] wrote:
URL: http://svn.apache.org/viewvc?rev=729538&view=rev
Hm. Why this rather complex approach with the request_status hook?
Why not doing the subrequest here or even better just inserting the
file buckets into the brigade right here and be done?
The whole point of the sendfile stuff is to free the backend socket as
early as possible and leave the rest to the apache. So I'm going out of
the proxy loop and handle everything in the request status hook, which
runs after the cleanup.
For freeing the backend socket it is IMHO possible to call
ap_proxy_release_connection first and do the subrequest run afterwards in
scgi_handler. This avoids the need for using the request_status hook
which makes the code IMHO more complex then needed.
Hmm. But I still have the internal redirect code there as well. I'd rather leave that as-is for the sake of separating concerns. If you ask me, I think, it's more simple now ;-)

IMHO this could be moved to the same location as well :-).

Also I don't want to duplicate the file delivery logic from core
(including sendfile, mmap etc), so putting file buckets myself into the
stream sounds not like the way to go.
Hm, there isn't that much basic logic to duplicate, basicly

            e = apr_brigade_insert_file(bb, fd, 0, r->finfo.size,
r->pool);

#if APR_HAS_MMAP
            if (d->enable_mmap == ENABLE_MMAP_OFF) {
                (void)apr_bucket_file_enable_mmap(e, 0);
            }
#endif
Isn't that private config?

At least it is one of another module. And yes I agree if there is no official
API to get the setting of d->enable_mmap (currently too lazy to check this) it
is the best to stay with the subrequest approach.

This is a general flaw with hoe things like EnableMMAP and EnableSendfile work -- we really should have an API that passes in a filepath to test for each attribute, because right now when you insert the buckets into the brigade, you need to enable sendfile and mmap on creation, and the core filters won't disable them even if they are disabled.

-Paul

Reply via email to