httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yle <lists....@gmail.com>
Subject Re: [PATCH] mod_proxy sessions
Date Mon, 01 Feb 2010 17:48:23 GMT
I reply in place ...

2010/2/1 Graham Leggett <minfrin@sharp.fm>

> On 01 Feb 2010, at 6:09 PM, yle lists wrote:
>
>  I'd like to submit a patch (against 2.2.14) that provides mod_proxy
>> sessions handling, that is, the ability to bind backend connections
>> (resources) to a session-id.
>>
>> When proxy sessions are enabled, the backend connections are recycled in a
>> per-session reslist.
>> The worker uses a hashmap for the acquired sessions, and recycles the
>> released ones in a reslist.
>>
>> One can use the sessions to limit, per-client (provided the client can be
>> identified, by IP or cookie or whatever), the number of connections to the
>> backend, or to forward statefull over-http-protocols.
>>
>> The session-id used for a particular request is given by the
>> "proxy-session-id" note (r->notes), so that the other modules (or a
>> Rewrite/SetEnv rule) can set it to a client identifier.
>>
>> Another note ("proxy-session-client", true/false) can be used to bind the
>> session to the client connection (the connection-id is the session-id), like
>> mod_proxy 2.0.
>> I use it to forward client NTLM connections, since the backend connections
>> will be per-client connection.
>>
>> For the sessions to be enabled, one can use the worker parameter
>> SessionsCache=on, and the SessionsCache(S)Max|Min parameters control the
>> sessions reslist.
>> The Session(S)Max|Min parameters control the number of connections per
>> session.
>>
>> Finally, the patch also includes the functions apr_reslist_create_ex and
>> apr_reslist_clear which would allow one to clear a reslist with a given
>> clearor function.
>> I also included comments in the patch regarding the destructor of the
>> proxy connection which, IMHO, is broken in httpd-2.2x.
>>
>
> Some comments based on a quick once over.
>
> Many small patches are easier to review than one big patch. Small patches
> are likely to be accepted quickly, leaving time for issues with other
> patches to be resolved.
>
> Changes to APR should be packaged separately and submitted to the APR
> project, httpd and APR split from each other some time ago, and httpd
> doesn't keep it's own private changes to APR. The extensions you propose to
> reslist look interesting, and are definitely worth picking up there.
>
> The patch should be to trunk rather than httpd v2.2, as all changes must be
> made to the trunk first before being considered for backport to v2.2.
>

OK, I can split the patch and adapt it to trunk if one looks interested.


> Your changes to ap_proxy_acquire_connection() and friends changes the ABI
> of the code, and as such cannot be backported as is to v2.2 as it stands.
>

Cf. explanation on ap_proxy_acquire_connection() ABI change below.


> Another potential issue is that this patch extends mod_proxy instead of
> adding a new module called mod_proxy_session. mod_proxy already contains too
> much non-abstracted code, and this makes it worse. Ideally, the bulk of the
> code should be in a module of its own.
>

Yes, it would be a better to use a module of its own, I have to better look
at the proxy hooks to see how the proxy_session module could do the job
elsewhere (most of the code would be that of the http module I guess, but I
need to look closer).
My first goal was to make it work ...


> I also don't fully see why the ABI of ap_proxy_acquire_connection() and
> friends was changed, can you clarify what you were trying to do?
>

I wanted ap_proxy_acquire_connection() be able to access the r->notes and
r->connection (for the session-id/client to be dynamically set-able), the
server record was of no help for me.

Maybe with a separate module I could use a new ap_proxy_acquire_session()
instead, let me think a bit on that.


> Regards,
> Graham
> --
>
> Thank you for your comments,
regards,
Yann.

Mime
View raw message