You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by jean-frederic clere <jf...@gmail.com> on 2007/06/26 18:46:21 UTC

Re: svn commit: r550519 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy.html.en docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_balancer.c

Ruediger Pluem wrote:
> Ok, partly playing a bit of devils advocate below :-).
>
> On 06/25/2007 04:42 PM, jfclere@apache.org wrote:
>   
>> Author: jfclere
>> Date: Mon Jun 25 07:42:25 2007
>> New Revision: 550519
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=550519
>> Log:
>> Add sticky_path to solve PR41897.
>>
>> Modified:
>>     httpd/httpd/trunk/docs/manual/mod/mod_proxy.html.en
>>     httpd/httpd/trunk/docs/manual/mod/mod_proxy.xml
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>     httpd/httpd/trunk/modules/proxy/mod_proxy.h
>>     httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>
>>     
>
>
>   
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.h?view=diff&rev=550519&r1=550518&r2=550519
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Mon Jun 25 07:42:25 2007
>> @@ -365,6 +365,7 @@
>>      apr_thread_mutex_t  *mutex;  /* Thread lock for updating lb params */
>>  #endif
>>      void            *context;   /* general purpose storage */
>> +    const char      *sticky_path;  /* URL sticky session identifier */
>>  };
>>     
>
> I am missing a minor bump, since this is part of a public API.
>
> Ok, furthermore I think we need to adjust the proxy_status_hook to
> actually display the string the user configured and not only the path
> for the cookie. The same is true for the balancer manager (display wise).
> Missing to split the string at the | in the balancer manager when you
> enter new data for sticky does not really worry me because setting
> a new sticky for the balancer (like all other balancer parameters)
> does not work anyway. We should really disable this.
>   
How? Keep the balancer-manager page as it is but "disable" that part. 
(form with fields read only and submit disabled).

Cheers

Jean-Frederic
> Just to avoid confusion: Changing parameters for a worker via the balancer
> manager works *well* (all this is stored in shared memory).
> Changing parameters for a balancer itself does *not* work as the balancer
> configuration is not stored in shared memory and thus the change only
> happens in that process that processed the according GET request to the
> balancer manager.
>
> Doing the right thing with the note "session-sticky" and the environment
> variable "BALANCER_SESSION_STICKY" seems to be a bit more tricky to me.
> At the point of time we are currently setting these we have already
> "forgotten" what we used for the route (sticky / sticky_path).
> OTOH we currently set "session-sticky"  and "BALANCER_SESSION_STICKY" only
> in the case that we really find a worker. We do not set them if all workers
> for this route / all workers in general are in error state. So moving
> this inside find_session_route would change this behaviour.
> But unseting the note and the env variable in the case of all workers being in
> error state seems to be the perfect trigger in a few month for the question:
> "Why the hell are we doing this?".
> So a change in behavior might be the better option here.
>
> As said: Devils advocate :-).
>
>
>
> Regards
>
> Rüdiger
>
>
>   


Re: svn commit: r550519 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy.html.en docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_balancer.c

Posted by jean-frederic clere <jf...@gmail.com>.
Ruediger Pluem wrote:
> On 06/26/2007 06:46 PM, jean-frederic clere wrote:
>   
>> Ruediger Pluem wrote:
>>     
>
>   
>>> Ok, furthermore I think we need to adjust the proxy_status_hook to
>>> actually display the string the user configured and not only the path
>>> for the cookie. The same is true for the balancer manager (display wise).
>>> Missing to split the string at the | in the balancer manager when you
>>> enter new data for sticky does not really worry me because setting
>>> a new sticky for the balancer (like all other balancer parameters)
>>> does not work anyway. We should really disable this.
>>>   
>>>       
>> How? Keep the balancer-manager page as it is but "disable" that part.
>> (form with fields read only and submit disabled).
>>     
>
> I am thinking more about removing it completely like in the attached patch, because
>
> 1. All information is also displayed without the form.
> 2. If it turns out that we need the form for the balancer again, because we adjusted
>    the balancer data structures in a way that changes via the balancer-manager work,
>    we can fetch it back from svn. IMHO no need to keep "dead" code.
>   
Ok I will commit your patch together with what was missing in r550519.

Cheers
Jean-Frederic
>
> Regards
>
> Rüdiger
>
>
>
>   


Re: svn commit: r550519 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy.html.en docs/manual/mod/mod_proxy.xml modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_balancer.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 06/26/2007 06:46 PM, jean-frederic clere wrote:
> Ruediger Pluem wrote:

>> Ok, furthermore I think we need to adjust the proxy_status_hook to
>> actually display the string the user configured and not only the path
>> for the cookie. The same is true for the balancer manager (display wise).
>> Missing to split the string at the | in the balancer manager when you
>> enter new data for sticky does not really worry me because setting
>> a new sticky for the balancer (like all other balancer parameters)
>> does not work anyway. We should really disable this.
>>   
> How? Keep the balancer-manager page as it is but "disable" that part.
> (form with fields read only and submit disabled).

I am thinking more about removing it completely like in the attached patch, because

1. All information is also displayed without the form.
2. If it turns out that we need the form for the balancer again, because we adjusted
   the balancer data structures in a way that changes via the balancer-manager work,
   we can fetch it back from svn. IMHO no need to keep "dead" code.


Regards

Rüdiger