You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mt...@apache.org on 2008/08/08 09:36:34 UTC

svn commit: r683892 - in /tomcat/connectors/trunk/jk: native/common/ xdocs/miscellaneous/ xdocs/reference/

Author: mturk
Date: Fri Aug  8 00:36:33 2008
New Revision: 683892

URL: http://svn.apache.org/viewvc?rev=683892&view=rev
Log:
Add connection_keepalive for sending CPING/CPONG over unused connections in connection pool

Modified:
    tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c
    tomcat/connectors/trunk/jk/native/common/jk_ajp_common.h
    tomcat/connectors/trunk/jk/native/common/jk_lb_worker.c
    tomcat/connectors/trunk/jk/native/common/jk_lb_worker.h
    tomcat/connectors/trunk/jk/native/common/jk_util.c
    tomcat/connectors/trunk/jk/native/common/jk_util.h
    tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml
    tomcat/connectors/trunk/jk/xdocs/reference/workers.xml

Modified: tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c Fri Aug  8 00:36:33 2008
@@ -2486,6 +2486,9 @@
             jk_get_worker_prepost_timeout(props, p->name,
                                           AJP_DEF_PREPOST_TIMEOUT);
 
+        p->connection_keepalive =
+            jk_get_worker_connection_keepalive(props, p->name, 0);
+
         p->recovery_opts =
             jk_get_worker_recovery_opts(props, p->name,
                                         AJP_DEF_RECOVERY_OPTS);
@@ -2826,7 +2829,8 @@
         jk_shm_unlock();
 
         /* Obtain current time only if needed */
-        if (aw->cache_timeout <= 0) {
+        if (aw->cache_timeout <= 0 &&
+            aw->connection_keepalive <= 0) {
             /* Nothing to do. */
             JK_TRACE_EXIT(l);
             return JK_TRUE;
@@ -2834,7 +2838,7 @@
 
         JK_ENTER_CS(&aw->cs, rc);
         if (rc) {
-            unsigned int n = 0, cnt = 0;
+            unsigned int n = 0, k = 0, cnt = 0;
             int i;
             /* Count open slots */
             for (i = (int)aw->ep_cache_sz - 1; i >= 0; i--) {
@@ -2842,7 +2846,8 @@
                     cnt++;
             }
             /* Handle worker cache and recycle timeouts */
-            for (i = (int)aw->ep_cache_sz - 1; i >= 0; i--) {
+            for (i = (int)aw->ep_cache_sz - 1;
+                 i >= 0 && aw->cache_timeout > 0; i--) {
                 /* Skip the closed sockets */
                 if (aw->ep_cache[i] && IS_VALID_SOCKET(aw->ep_cache[i]->sd)) {
                     int elapsed = (int)difftime(now, aw->ep_cache[i]->last_access);
@@ -2868,12 +2873,47 @@
                     break;
                 }
             }
+            /* Handle worker connection keepalive */
+            for (i = (int)aw->ep_cache_sz - 1; i >= 0 &&
+                 aw->connection_keepalive > 0 &&
+                 aw->prepost_timeout > 0; i--) {
+                /* Skip the closed sockets */
+                if (aw->ep_cache[i] && IS_VALID_SOCKET(aw->ep_cache[i]->sd)) {
+                    int elapsed = (int)difftime(now, aw->ep_cache[i]->last_access);
+                    if (elapsed > aw->connection_keepalive) {
+                        k++;
+                        /* handle cping/cpong.
+                         */
+                        if (ajp_handle_cping_cpong(aw->ep_cache[i],
+                            aw->prepost_timeout, l) == JK_FALSE) {
+                            jk_log(l, JK_LOG_INFO,
+                                   "(%s) failed sending request, "
+                                   "socket %d keepalive cping/cpong "
+                                   "failure (errno=%d)",
+                                   aw->name,
+                                   aw->ep_cache[i]->sd,
+                                   aw->ep_cache[i]->last_errno);
+                            aw->ep_cache[i]->reuse = JK_FALSE;
+                            ajp_reset_endpoint(aw->ep_cache[i], l);
+                        }
+                        else {
+                            now = time(NULL);
+                            aw->ep_cache[i]->last_access = now;
+                        }
+                    }
+                }
+            }
             JK_LEAVE_CS(&aw->cs, rc);
-            if (JK_IS_DEBUG_LEVEL(l))
+            if (n && JK_IS_DEBUG_LEVEL(l))
                 jk_log(l, JK_LOG_DEBUG,
                         "recycled %u sockets in %d seconds from %u pool slots",
                         n, (int)(difftime(time(NULL), now)),
                         aw->ep_cache_sz);
+            if (k && JK_IS_DEBUG_LEVEL(l))
+                jk_log(l, JK_LOG_DEBUG,
+                        "pinged %u sockets in %d seconds from %u pool slots",
+                        k, (int)(difftime(time(NULL), now)),
+                        aw->ep_cache_sz);
             JK_TRACE_EXIT(l);
             return JK_TRUE;
         }

Modified: tomcat/connectors/trunk/jk/native/common/jk_ajp_common.h
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_ajp_common.h?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_ajp_common.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_ajp_common.h Fri Aug  8 00:36:33 2008
@@ -311,6 +311,8 @@
     int connect_timeout;   /* connect cping/cpong delay in ms (0 means disabled)  */
     int reply_timeout;     /* reply timeout delay in ms (0 means disabled) */
     int prepost_timeout;   /* before sending a request cping/cpong timeout delay in ms (0 means disabled) */
+    int connection_keepalive; /* interval for sending cping packets on
+                               * unused connection */
 
     /*
      * Recovery options

Modified: tomcat/connectors/trunk/jk/native/common/jk_lb_worker.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_lb_worker.c?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_lb_worker.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_lb_worker.c Fri Aug  8 00:36:33 2008
@@ -1614,6 +1614,10 @@
     if(p->maintain_time < 0)
         p->maintain_time = 0;
     p->s->last_maintain_time = time(NULL);
+    p->connection_keepalive = jk_get_worker_connection_keepalive(props,
+                                                                 p->name, 0);
+    if(p->connection_keepalive < 0)
+        p->connection_keepalive = 0;
 
     p->lbmethod = jk_get_lb_method(props, p->name);
     p->lblock   = jk_get_lb_lock(props, p->name);

Modified: tomcat/connectors/trunk/jk/native/common/jk_lb_worker.h
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_lb_worker.h?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_lb_worker.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_lb_worker.h Fri Aug  8 00:36:33 2008
@@ -192,6 +192,7 @@
     int          lbmethod;
     int          lblock;
     int          maintain_time;
+    int          connection_keepalive;
     unsigned int max_packet_size;
     unsigned int next_offset;
 

Modified: tomcat/connectors/trunk/jk/native/common/jk_util.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_util.c?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_util.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_util.c Fri Aug  8 00:36:33 2008
@@ -59,6 +59,7 @@
 #define SOCKET_TIMEOUT_OF_WORKER    ("socket_timeout")
 #define SOCKET_BUFFER_OF_WORKER     ("socket_buffer")
 #define SOCKET_KEEPALIVE_OF_WORKER  ("socket_keepalive")
+#define CONNECTION_KEEPALIVE_OF_WORKER  ("connection_keepalive")
 #define RECYCLE_TIMEOUT_DEPRECATED  ("recycle_timeout")
 #define LOAD_FACTOR_OF_WORKER       ("lbfactor")
 #define DISTANCE_OF_WORKER          ("distance")
@@ -173,6 +174,7 @@
     SOCKET_TIMEOUT_OF_WORKER,
     SOCKET_BUFFER_OF_WORKER,
     SOCKET_KEEPALIVE_OF_WORKER,
+    CONNECTION_KEEPALIVE_OF_WORKER,
     RECYCLE_TIMEOUT_DEPRECATED,
     LOAD_FACTOR_OF_WORKER,
     STICKY_SESSION,
@@ -256,6 +258,7 @@
     SOCKET_TIMEOUT_OF_WORKER,
     SOCKET_BUFFER_OF_WORKER,
     SOCKET_KEEPALIVE_OF_WORKER,
+    CONNECTION_KEEPALIVE_OF_WORKER,
     RECYCLE_TIMEOUT_DEPRECATED,
     LOAD_FACTOR_OF_WORKER,
     DISTANCE_OF_WORKER,
@@ -947,6 +950,19 @@
     return jk_map_get_bool(m, buf, def);
 }
 
+int jk_get_worker_connection_keepalive(jk_map_t *m, const char *wname, int def)
+{
+    char buf[1024];
+
+    if (!m || !wname) {
+        return -1;
+    }
+
+    MAKE_WORKER_PARAM(CONNECTION_KEEPALIVE_OF_WORKER);
+
+    return jk_map_get_int(m, buf, def);
+}
+
 int jk_get_worker_cache_timeout(jk_map_t *m, const char *wname, int def)
 {
     char buf[1024];

Modified: tomcat/connectors/trunk/jk/native/common/jk_util.h
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_util.h?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_util.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_util.h Fri Aug  8 00:36:33 2008
@@ -74,6 +74,8 @@
 
 int jk_get_worker_socket_keepalive(jk_map_t *m, const char *wname, int def);
 
+int jk_get_worker_connection_keepalive(jk_map_t *m, const char *wname, int def);
+
 int jk_get_worker_cache_timeout(jk_map_t *m, const char *wname, int def);
 
 int jk_get_worker_recovery_opts(jk_map_t *m, const char *wname, int def);

Modified: tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml (original)
+++ tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml Fri Aug  8 00:36:33 2008
@@ -43,6 +43,9 @@
   <br />
   <subsection name="Native">
     <changelog>
+      <update>
+        Added connection_keepalive directive. (mturk)
+      </update>
       <fix>
         Documentation: "val" attribute numbering in status worker
         needs to start with 0 instead of 1. (rjung)

Modified: tomcat/connectors/trunk/jk/xdocs/reference/workers.xml
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/xdocs/reference/workers.xml?rev=683892&r1=683891&r2=683892&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/xdocs/reference/workers.xml (original)
+++ tomcat/connectors/trunk/jk/xdocs/reference/workers.xml Fri Aug  8 00:36:33 2008
@@ -260,6 +260,17 @@
 </p>
 </directive>
 
+<directive name="connection_keepalive" default="False" required="false">
+This is interval in seconds used for sending CPING packets over unused
+connection.
+<p>To be able to use this feature <code>prepost_timeout</code> must be
+set to desired timeout value.
+</p>
+<p>
+This feature has been added in <b>jk 1.2.27</b>.
+</p>
+</directive>
+
 <directive name="connection_pool_size" default="see text" required="false">
 This defines the number of connections made to the AJP backend that
 are maintained as a connection pool.



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


Re: JK connection_keepalive configuration

Posted by Rainer Jung <ra...@kippdata.de>.
Mladen Turk schrieb:
> Rainer Jung wrote:
>> Hi Mladen,
>>
>> any thoughts, how we could make configuring connection_keepalive better?
>>
>> Of course by introducing even more attributes we can keep the individual
>> CPING timeouts, but I don't really see a reason. I want to get the
>> attributes right, before we release this nice feature.
>>
>> Comments?
>>
> 
> We must not change the existing behavior nor directives.
> They are widely used, so IMO, we can only use the
> additional directive (probe_timeout is fine),
> although ping_timeout is more intuitive.

I didn't want to change the behaviour. But if we want to make people
aware of the new (and better) attributes, we can always add attributes
to the deprecated_properties in jk_util.c. The only thing this does, is
logging a warning during startup. As long as we keep the code handling
the attributes (and of course we will keep it during 1.2.x), the
functionality wil still be there.

> We can then use that ping_timeout for connect_timeout
> if the connect_timeout value is boolean (allowing booleans first)
> The same for prepost_timeout.

That works.

> so.
> worker.x.ping_timeout=NNN
> worker.x.connect_timeout=True|On|1 (will set to ping_timeout)
> worker.x.prepost_timeout=True|On|1 (will set to ping_timeout)
> 
> If you like we could use one additional
> worker.x.ping_mode=connect,prepost,keepalive
> I'd like we use single letters here 'CPK'

I like this much better. Letters are fine, I would allow short form
(letters) and long form (easier to understand in the configuration
file), like we do for activation. We could also add "a=all" meaning
whatever cping probes are there now and in the future.

I would suggest we use a non zero default value for ping_timeout, like
e.g. 10 seconds. That way most people only need to activate the type of
timeout wanted via ping_mode and need not care about the timeout value
itself. Code for setting internally prepost and connect timeouts gets
1-2 lines longer, because we can't simply take ping_timeout as the
default, but the user experience will be better (e.g. if the set
ping_mode to use pings, but don't give any timeouts?).

People only using the old attributes prepost_timeout and connect_timeout
will not experience any differing behaviour.

> Also I don't like we deprecate *_timeout directives.

Even if it only means "log a warning"?

> Next,
> connection_keepalive should probably get
> renamed to connection_keepalive_interval

... or maybe we get rid of "keepalive", because there is already so much
confusion between TCP and HTTP keepalive (and now also AJP). What about
connection_ping_interval or connection_test_interval?

I saw you already started with some changes, I can add some of the rest
tomorrow evening.

Regards,

Rainer

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


Re: JK connection_keepalive configuration

Posted by Mladen Turk <mt...@apache.org>.
Rainer Jung wrote:
> Hi Mladen,
> 
> any thoughts, how we could make configuring connection_keepalive better?
> 
> Of course by introducing even more attributes we can keep the individual
> CPING timeouts, but I don't really see a reason. I want to get the
> attributes right, before we release this nice feature.
> 
> Comments?
>

We must not change the existing behavior nor directives.
They are widely used, so IMO, we can only use the
additional directive (probe_timeout is fine),
although ping_timeout is more intuitive.

We can then use that ping_timeout for connect_timeout
if the connect_timeout value is boolean (allowing booleans first)
The same for prepost_timeout.

so.
worker.x.ping_timeout=NNN
worker.x.connect_timeout=True|On|1 (will set to ping_timeout)
worker.x.prepost_timeout=True|On|1 (will set to ping_timeout)

If you like we could use one additional
worker.x.ping_mode=connect,prepost,keepalive
I'd like we use single letters here 'CPK'

Also I don't like we deprecate *_timeout directives.

Next,
connection_keepalive should probably get
renamed to connection_keepalive_interval


Cheers
-- 
^(TM)

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


JK connection_keepalive configuration

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Mladen,

any thoughts, how we could make configuring connection_keepalive better?

We implicitely use prepost_timeout, so there's no way to use the
occasional keepalive CPING probing without adding CPINGs to any request.

I think in most cases, the actual timeout used for the CPONG response in
the three cases connection check, pre request check and keepalive check
do not matter that much, they usually can be set to the same value.

So we could introduce a more general probe_timeout and a flag use_probes
indicating, which types of probes are wanted (connect, request, idle).

The probe_timeout will be copied to prepost_timeout resp.
connect_timeout depending on the flag value and prepost_timeout and
connect_timeout will get deprecated (but still used if not overwritten
by the new directives).

The flag could be a bitmap, but I would prefer we make it a list
attribute with string arguments ("connect", "request", "idle").

By this we would offer the alternative

   probe_timeout, use_probes, connection_keepalive

to

   prepost_timeout, connect_timeout, connection_keepalive

We would loose the ability to set the CPONG timeouts individually, but
we gain the ability to use keepalive CPINGs without prepost CPINGs.

Of course by introducing even more attributes we can keep the individual
CPING timeouts, but I don't really see a reason. I want to get the
attributes right, before we release this nice feature.

Comments?

Regards,

Rainer

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