Re: svn commit: r1799498 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xm

[Christopher Schultz]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

All,

Any objections to me back-porting this (and r1799677 as well) at least
back to 8.0?

Thanks,
- -chris

On 6/21/17 3:05 PM, schu...@apache.org wrote:
> Author: schultz Date: Wed Jun 21 19:05:38 2017 New Revision:
> 1799498
> 
> URL: http://svn.apache.org/viewvc?rev=1799498=rev Log: Add
> LoadBalancerDrainingValve.
> 
> Added: 
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve
.java
> (with props) 
> tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingV
alve.java
> (with props) Modified: tomcat/trunk/webapps/docs/changelog.xml 
> tomcat/trunk/webapps/docs/config/valve.xml
> 
> Added:
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve
.java
>
> 
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valve
s/LoadBalancerDrainingValve.java?rev=1799498=auto
> ==

>
> 
- ---
tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.j
ava
(added)
> +++
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve
.java
> Wed Jun 21 19:05:38 2017 @@ -0,0 +1,277 @@ +/* + * Licensed to the
> Apache Software Foundation (ASF) under one or more + * contributor
> license agreements.  See the NOTICE file distributed with + * this
> work for additional information regarding copyright ownership. + *
> The ASF licenses this file to You under the Apache License, Version
> 2.0 + * (the "License"); you may not use this file except in
> compliance with + * the License.  You may obtain a copy of the
> License at + * + *  http://www.apache.org/licenses/LICENSE-2.0 
> + * + * Unless required by applicable law or agreed to in writing,
> software + * distributed under the License is distributed on an "AS
> IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
> express or implied. + * See the License for the specific language
> governing permissions and + * limitations under the License. + */ 
> +package org.apache.catalina.valves; + +import
> java.io.IOException; + +import javax.servlet.ServletException; 
> +import javax.servlet.http.Cookie; +import
> javax.servlet.http.HttpServletResponse; + +import
> org.apache.catalina.LifecycleException; +import
> org.apache.catalina.connector.Request; +import
> org.apache.catalina.connector.Response; +import
> org.apache.catalina.util.SessionConfig; +import
> org.apache.juli.logging.Log; + +/** + * A Valve to detect
> situations where a load-balanced node receiving a + * request has
> been deactivated by the load balancer (JK_LB_ACTIVATION=DIS) + *
> and the incoming request has no valid session. + * + * In
> these cases, the user's session cookie should be removed if it
> exists, + * any ";jsessionid" parameter should be removed from the
> request URI, + * and the client should be redirected to the same
> URI. This will cause the + * load-balanced to re-balance the client
> to another server. + * + * A request parameter is added to
> the redirect URI in order to avoid + * repeated redirects in the
> event of an error or misconfiguration. + * + * All this work
> is required because when the activation state of a node is + *
> DISABLED, the load-balancer will still send requests to the node if
> they + * appear to have a session on that node. Since mod_jk
> doesn't actually know + * whether the session id is valid, it will
> send the request blindly to + * the disabled node, which makes it
> take much longer to drain the node + * than strictly
> necessary. + * + * For testing purposes, a special cookie
> can be configured and used + * by a client to ignore the normal
> behavior of this Valve and allow + * a client to get a new session
> on a DISABLED node. See + * {@link #setIgnoreCookieName} and {@link
> #setIgnoreCookieValue} + * to configure those values. + * + *
> This Valve should be installed earlier in the Valve pipeline
> than any + * authentication valves, as the redirection should take
> place before an + * authentication valve would save a request to a
> protected resource. + * + * @see
> http://tomcat.apache.org/connectors-doc/generic_howto/loadbalancers.ht
ml
>
> 
+ */
> +public class LoadBalancerDrainingValve +extends ValveBase +{ +
> /** + * The request attribute key where the load-balancer's
> activation state + * can be found. + */ +static final
> String ATTRIBUTE_KEY_JK_LB_ACTIVATION = "JK_LB_ACTIVATION"; + +
> /** + * The HTTP response code that will be used to redirect
> the request + * back to the load-balancer for re-balancing.
> Defaults to 307 + * (TEMPORARY_REDIRECT). + * + * HTTP
> status code 305 (USE_PROXY) might be an option, here. too. +
> */ +private int _redirectStatusCode =
> HttpServletResponse.SC_TEMPORARY_REDIRECT; + +/** + * The
> name of the cookie which can be set to ignore the "draining"
> action + * of this Filter. This will allow a client to contact
> 

Re: svn commit: r1799498 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xm

[Christopher Schultz]

Martin,

On 6/22/17 3:04 AM, Martin Grigorov wrote:
> On Wed, Jun 21, 2017 at 9:05 PM,  wrote:
>
>> +/**
>> + * The request attribute key where the load-balancer's activation
>> state
>> + * can be found.
>> + */
>> +static final String ATTRIBUTE_KEY_JK_LB_ACTIVATION =
>> "JK_LB_ACTIVATION";
>>
> > Any objection to make this constant public and visible from outside
> ? I find it useful to be able to refer the constant by name than its
> value when integrating.

No objections. It looks like I'll have a follow-on patch shortly, so I
can change this at the same time.

>> +// Kill any session cookie present
>> +if(null != cookies) {
>> +for(Cookie cookie : cookies) {
>> +final String cookieName = cookie.getName();
>> +if(containerLog.isTraceEnabled())
>> +containerLog.trace("Checking cookie " +
>> cookieName + "=" + cookie.getValue());
>> +
>> +if(sessionCookieName.equals(cookieName)
>> +   &&
request.getRequestedSessionId().equals(cookie.getValue()))
>> {
>> +sessionCookie = cookie;
>>
>
> Is it a good idea to 'break' here ?
> Do you expect more than one session cookies ?!

No, but I do expect that there may be more interesting cookies later on
in the list...

>> +} else
>> +// Is the client presenting a valid ignore-cookie
>> value?
>> +if(null != _ignoreCookieName
>> +&& _ignoreCookieName.equals(cookieName)
>> +&& null != _ignoreCookieValue
>> +&&
_ignoreCookieValue.equals(cookie.getValue()))
>> {
>> +ignoreRebalance = true;
>> +}
>> +}

Like here ^.

>> +// Re-write the URI if it contains a ;jsessionid parameter
>> +String uri = request.getRequestURI();
>> +String sessionURIParamName = "jsessionid";
>> +SessionConfig.getSessionUriParamName(request.getContext());
>>
>
> It seems this bug has been inroduced during testing/debugging.
> The return value of
> "SessionConfig.getSessionUriParamName(request.getContext());"
> is ignored and the sessionURIParamName is always "jsessionid".

+1

I'll get that fixed. This Valve is a port of a Filter that I wrote
earlier and evidently that got lost in the shuffle.

Thanks for the review.

-chris



signature.asc
Description: OpenPGP digital signature

Re: svn commit: r1799498 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xm

[Martin Grigorov]

Hi Chris,


On Wed, Jun 21, 2017 at 9:05 PM,  wrote:

> Author: schultz
> Date: Wed Jun 21 19:05:38 2017
> New Revision: 1799498
>
> URL: http://svn.apache.org/viewvc?rev=1799498=rev
> Log:
> Add LoadBalancerDrainingValve.
>
> Added:
> 
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
>  (with props)
> tomcat/trunk/test/org/apache/catalina/valves/
> TestLoadBalancerDrainingValve.java   (with props)
> Modified:
> tomcat/trunk/webapps/docs/changelog.xml
> tomcat/trunk/webapps/docs/config/valve.xml
>
> Added: tomcat/trunk/java/org/apache/catalina/valves/
> LoadBalancerDrainingValve.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
> catalina/valves/LoadBalancerDrainingValve.java?rev=1799498=auto
> 
> ==
> --- 
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
> (added)
> +++ 
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
> Wed Jun 21 19:05:38 2017
> @@ -0,0 +1,277 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *  http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.catalina.valves;
> +
> +import java.io.IOException;
> +
> +import javax.servlet.ServletException;
> +import javax.servlet.http.Cookie;
> +import javax.servlet.http.HttpServletResponse;
> +
> +import org.apache.catalina.LifecycleException;
> +import org.apache.catalina.connector.Request;
> +import org.apache.catalina.connector.Response;
> +import org.apache.catalina.util.SessionConfig;
> +import org.apache.juli.logging.Log;
> +
> +/**
> + * A Valve to detect situations where a load-balanced node receiving a
> + * request has been deactivated by the load balancer
> (JK_LB_ACTIVATION=DIS)
> + * and the incoming request has no valid session.
> + *
> + * In these cases, the user's session cookie should be removed if it
> exists,
> + * any ";jsessionid" parameter should be removed from the request URI,
> + * and the client should be redirected to the same URI. This will cause
> the
> + * load-balanced to re-balance the client to another server.
> + *
> + * A request parameter is added to the redirect URI in order to avoid
> + * repeated redirects in the event of an error or misconfiguration.
> + *
> + * All this work is required because when the activation state of a
> node is
> + * DISABLED, the load-balancer will still send requests to the node if
> they
> + * appear to have a session on that node. Since mod_jk doesn't actually
> know
> + * whether the session id is valid, it will send the request blindly to
> + * the disabled node, which makes it take much longer to drain the node
> + * than strictly necessary.
> + *
> + * For testing purposes, a special cookie can be configured and used
> + * by a client to ignore the normal behavior of this Valve and allow
> + * a client to get a new session on a DISABLED node. See
> + * {@link #setIgnoreCookieName} and {@link #setIgnoreCookieValue}
> + * to configure those values.
> + *
> + * This Valve should be installed earlier in the Valve pipeline than
> any
> + * authentication valves, as the redirection should take place before an
> + * authentication valve would save a request to a protected resource.
> + *
> + * @see http://tomcat.apache.org/connectors-doc/generic_howto/
> loadbalancers.html
> + */
> +public class LoadBalancerDrainingValve
> +extends ValveBase
> +{
> +/**
> + * The request attribute key where the load-balancer's activation
> state
> + * can be found.
> + */
> +static final String ATTRIBUTE_KEY_JK_LB_ACTIVATION =
> "JK_LB_ACTIVATION";
>

Any objection to make this constant public and visible from outside ?
I find it useful to be able to refer the constant by name than its value
when integrating.


> +
> +/**
> + * The HTTP response code that will be used to redirect the request
> + * back to the load-balancer for re-balancing. Defaults to 307
> + * (TEMPORARY_REDIRECT).
> + *
> + * HTTP status code 305 (USE_PROXY) might be an option, here. too.
> + */
> +private int _redirectStatusCode = HttpServletResponse.SC_
> TEMPORARY_REDIRECT;
> +
> +/**
> + * 

Re: svn commit: r1799498 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xm

[Mark Thomas]

On 21/06/17 20:05, schu...@apache.org wrote:
> Author: schultz
> Date: Wed Jun 21 19:05:38 2017
> New Revision: 1799498
> 
> URL: http://svn.apache.org/viewvc?rev=1799498=rev
> Log:
> Add LoadBalancerDrainingValve.
> 
> Added:
> 
> tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java   
> (with props)
> 
> tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
>(with props)
> Modified:
> tomcat/trunk/webapps/docs/changelog.xml
> tomcat/trunk/webapps/docs/config/valve.xml



> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1799498=1799497=1799498=diff
> ==
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Wed Jun 21 19:05:38 2017
> @@ -138,6 +138,10 @@
>  variable for CGI executables is populated in a consistent way 
> regardless
>  of how the CGI servlet is mapped to a request. (markt)
>
> +  
> +Add LoadBalancerDrainingValve, a Valve designed to reduce the amount 
> of
> +time required for a node to drain its authenticated users. (schultz)
> +  
>  
>
>

Wrong version again.

Mark

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org