Bug 36385

Summary: [mod_jk1.2.14] Prepost pings are not sent before the first request
Product: Tomcat Connectors Reporter: Edgar Alves <ealves>
Component: CommonAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   

Description Edgar Alves 2005-08-26 19:42:53 UTC
Prepost pings are not sent before the first request over a new connection.

  Here's a possible patch:

<---- START OF PATCH ---->
---
jakarta-tomcat-connectors-1.2.14.1-src-original/jk/native/common/jk_ajp_common.c
   2005-05-26 15:36:14.000000000 +0100
+++
jakarta-tomcat-connectors-1.2.14.1-src-modified/jk/native/common/jk_ajp_common.c
   2005-08-26 18:25:22.000000000 +0100
@@ -1229,7 +1229,11 @@
         }
         /* no need to handle cping/cpong here since it should be at connection
time */

-        if (ajp_connect_to_endpoint(ae, l) == JK_TRUE) {
+        if ((ajp_connect_to_endpoint(ae, l) == JK_TRUE) &&
+                ((ae->worker->connect_timeout > 0) ||
+                (!(ae->worker->prepost_timeout > 0)) ||
+                (ajp_handle_cping_cpong(ae, ae->worker->prepost_timeout, l)
+                    == JK_TRUE))) {
             /*
              * After we are connected, each error that we are going to
              * have is probably unrecoverable
<---- END OF PATCH ---->
Comment 1 Henri Gomez 2005-09-02 10:43:15 UTC
cping/cpong are handled in ajp_connect_to_endpoint
Comment 2 Edgar Alves 2005-09-02 11:33:10 UTC
(In reply to comment #1)
> cping/cpong are handled in ajp_connect_to_endpoint

  ajp_connect_to_endpoint sends a ping to validate the connection if
connect_timeout has been set.
  However, if you haven't set connect_timeout and you've set prepost_timeout
(asking for a ping to be sent before every request), no ping will be sent before
the first request.

  Here's an alternative patch (against mod_jk 1.2.14) that I hope better
illustrates what I'm trying to say:

<---- START OF PATCH ---->
--- jk_ajp_common.c	2005-09-02 10:22:18.446771900 +0100
+++ jk_ajp_common.c_modified	2005-09-02 10:25:36.164484700 +0100
@@ -881,6 +881,11 @@
                         ae->worker->connect_timeout, l);
             JK_TRACE_EXIT(l);
             return rc;
+        } else if (ae->worker->prepost_timeout > 0) {
+            rc = ajp_handle_cping_cpong (ae,
+                        ae->worker->prepost_timeout, l);
+            JK_TRACE_EXIT(l);
+            return rc;
         }
         JK_TRACE_EXIT(l);
         return JK_TRUE;
<---- END OF PATCH ---->

  I understand that I can get the same behaviour by setting connect_timeout to
the same value as prepost_timeout, but setting prepost_timeout should be enough
to have a ping sent before every request.
Comment 3 Henri Gomez 2005-09-02 11:46:07 UTC
Well I prefer :

         /* no need to handle cping/cpong here since it should be at connection
time */

-        if (ajp_connect_to_endpoint(ae, l) == JK_TRUE) {
+        if ((ajp_connect_to_endpoint(ae, l) == JK_TRUE) &&
+                ((ae->worker->connect_timeout < 0) &&
+                ((ae->worker->prepost_timeout > 0)) ||
+                (ajp_handle_cping_cpong(ae, ae->worker->prepost_timeout, l)
+                    == JK_TRUE))) {
             /*
              * After we are connected, each error that we are going to
              * have is probably unrecoverable


Don't send a ping before request if a ping as been send during connect !
Comment 4 Henri Gomez 2005-09-02 12:05:32 UTC
Index: jk_ajp_common.c
===================================================================
RCS file: /home/cvs//jakarta-tomcat-connectors/jk/native/common/jk_ajp_common.c,v
retrieving revision 1.119
diff -u -r1.119 jk_ajp_common.c
--- jk_ajp_common.c	26 May 2005 14:36:14 -0000	1.119
+++ jk_ajp_common.c	2 Sep 2005 10:04:17 -0000
@@ -1227,9 +1227,20 @@
             JK_TRACE_EXIT(l);
             return JK_FALSE;
         }
-        /* no need to handle cping/cpong here since it should be at connection
time */
+        
+        if ((ajp_connect_to_endpoint(ae, l) == JK_TRUE)) {
 
-        if (ajp_connect_to_endpoint(ae, l) == JK_TRUE) {
+           /* no need to handle cping/cpong if allready sent at connection time */
+
+           if ((ae->worker->connect_timeout <= 0) &&
(ae->worker->prepost_timeout > 0)) {
+             if (ajp_handle_cping_cpong(ae, ae->worker->prepost_timeout, l) !=
JK_TRUE) {
+                jk_log(l, JK_LOG_INFO,
+                       "Error sending ping pong on a fresh connection");
+                JK_TRACE_EXIT(l);
+                return JK_FALSE;
+             }
+           }
+           
             /*
              * After we are connected, each error that we are going to
              * have is probably unrecoverable

Here is my latest prefered patch.

BTW, I couldn't commit it (damnt eclipse)
Comment 5 Mladen Turk 2005-09-02 12:40:08 UTC
Sending cping/cpong makes no sense if connect was successful,
and it was deliberately removed from recent versions.
ajp_connect_to_enpoint uses timeout (socket_timeout), and
if it succeeds there is no need to try sending cping/cpong.
Tomcat will either hang or simply refuse the connection if
the max thread limit gets reached, so the first phase in that
case will fail.
IMO there is no point of sending cping/cpong if new connection
is established. It makes sense only with already connected sockets where
it is used to deal with broken sockets.


Comment 6 Edgar Alves 2005-09-02 13:08:04 UTC
Comments inline.

(In reply to comment #5)
> Sending cping/cpong makes no sense if connect was successful,
> and it was deliberately removed from recent versions.
> ajp_connect_to_enpoint uses timeout (socket_timeout), and
> if it succeeds there is no need to try sending cping/cpong.
> Tomcat will either hang or simply refuse the connection if
> the max thread limit gets reached, so the first phase in that
> case will fail.

  I've found this behaviour (which I consider a bug) when Enhydra/Tomcat had a
problem that allowed me to establish connections to the AJP port but wouldn't
respond to any requests (including the first one on a fresh connection).

> IMO there is no point of sending cping/cpong if new connection
> is established. It makes sense only with already connected sockets where
> it is used to deal with broken sockets.

  But that's the sole reason for the existence of the connect_timeout property:
for sending a cping/cpong after a new connection is established. What I think
that is a bug is the fact that if I use only prepost_timeout and not
connect_timeout, there is no cping/cpong before the first request (at least, if
this is the intended behavior, a remark should be make in the documentation).
Comment 7 Henri Gomez 2005-09-02 13:54:10 UTC
(In reply to comment #5)
> Sending cping/cpong makes no sense if connect was successful,
> and it was deliberately removed from recent versions.

> ajp_connect_to_enpoint uses timeout (socket_timeout), and
> if it succeeds there is no need to try sending cping/cpong.
> Tomcat will either hang or simply refuse the connection if
> the max thread limit gets reached, so the first phase in that
> case will fail.

I didn't agree. Even if you connect to the remote Tomcat, this one could be
hang and that was the ping/pong goal, detect a zombie or hanged tomcat.

I got case where the tomcat was able to do the accept but was in serious trouble
at a later time, and didn't process the request.

> IMO there is no point of sending cping/cpong if new connection
> is established. It makes sense only with already connected sockets where
> it is used to deal with broken sockets.

Comment 8 Remy Maucherat 2005-09-02 14:36:32 UTC
(In reply to comment #7)
> I didn't agree. Even if you connect to the remote Tomcat, this one could be
> hang and that was the ping/pong goal, detect a zombie or hanged tomcat.
> 
> I got case where the tomcat was able to do the accept but was in serious trouble
> at a later time, and didn't process the request.

Ok, but I don't see a way to guarantee that a Tomcat will process a request.
Answering a pong right after an accept does not add much.
Comment 9 Henri Gomez 2005-09-02 14:43:19 UTC
at least you could be sure some sort of code in the request handler is still
alive and running.

Not a 100% guarantee of course :(
Comment 10 Remy Maucherat 2005-09-02 16:53:37 UTC
(In reply to comment #9)
> at least you could be sure some sort of code in the request handler is still
> alive and running.

Yes, it means the acceptor thread has managed to allocate a processor thread. If
it can't it's supposed to close the socket, though, which should indicate the
Tomcat instance is "down".

If it does allocate the processor and starts doing things, I don't see how it
can fail to send a pong back. What is the Tomcat state where there would be a
problem ?

(BTW, I think I've fixed the biggest AJP APR bugs, so you can test it: if your
servers have resource efficiency issues, then it can't hurt)
Comment 11 Rainer Jung 2008-01-01 16:03:54 UTC
This will be fixed in version 1.2.27.