Author: rjung Date: Sun Sep 2 15:50:40 2007 New Revision: 572181 URL: http://svn.apache.org/viewvc?rev=572181&view=rev Log: - Document return codes of the service() method for all worker types (lb still missing). Below is for jk_ajp_common.c: - Document return codes of ajp_send_request() and ajp_get_reply(). - Fix log message typo - Don't use the recovery_options for idempotent GET and HEAD in case we already sent the headers. - We don't need to handle JK_FATAL_ERROR for ajp_process_callback() in ajp_get_reply(). - Handle JK_INTERNAL_ERROR instead of JK_SERVER_ERROR for ajp_process_callback() in ajp_get_reply(). - Add some logging for unexpected default case in return code handling. - Move complete handling of return codes of ajp_send_request() in front of return code handling for ajp_get_reply() in service(). - Some XXXs still open: return codes, is_error and op->recoverable for unexpected cases. We still need a better design for returning enough information from service() of a member to an lb, such that the lb can decide about recovery and if the member should be put in error state.
Modified: tomcat/connectors/trunk/jk/native/common/jk_ajp12_worker.c tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c tomcat/connectors/trunk/jk/native/common/jk_jni_worker.c tomcat/connectors/trunk/jk/native/common/jk_status.c Modified: tomcat/connectors/trunk/jk/native/common/jk_ajp12_worker.c URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_ajp12_worker.c?rev=572181&r1=572180&r2=572181&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/native/common/jk_ajp12_worker.c (original) +++ tomcat/connectors/trunk/jk/native/common/jk_ajp12_worker.c Sun Sep 2 15:50:40 2007 @@ -84,6 +84,19 @@ static int ajpv12_handle_request(ajp12_endpoint_t * p, jk_ws_service_t *s, jk_logger_t *l); +/* + * Return values of service() method for ajp12 worker: + * return value is_error reason + * JK_FALSE JK_HTTP_SERVER_ERROR Invalid parameters (null values) + * Error during connect to the backend + * ajpv12_handle_request() returns false: + * Any error during reading a request body from the client or + * sending the request to the backend + * JK_FALSE JK_HTTP_OK ajpv12_handle_response() returns false: + * Any error during reading parts of response from backend or + * sending to client + * JK_TRUE JK_HTTP_OK All other cases + */ static int JK_METHOD service(jk_endpoint_t *e, jk_ws_service_t *s, jk_logger_t *l, int *is_error) 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=572181&r1=572180&r2=572181&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c (original) +++ tomcat/connectors/trunk/jk/native/common/jk_ajp_common.c Sun Sep 2 15:50:40 2007 @@ -1178,6 +1178,19 @@ * * nb: reqmsg is the original request msg buffer * repmsg is the reply msg buffer which could be scratched + * + * Return values of ajp_send_request() function: + * return value op->recoverable reason + * JK_FALSE JK_FALSE/JK_TRUE ajp_connection_tcp_send_message() returns JK_FATAL_ERROR + * Endpoint belongs to unknown protocol. + * XXX: The recoverability handling of JK_FATAL_ERROR returning from + * ajp_connection_tcp_send_message() seems to be inconsistent. + * JK_FALSE JK_TRUE ajp_connection_tcp_send_message() returns JK_FALSE + * Sending data in jk_tcp_socket_sendfull() returns with error. + * JK_FALSE JK_TRUE Could not connect to backend + * JK_SERVER_ERROR JK_TRUE Error during sending parts of POST body to backend + * JK_CLIENT_RD_ERROR JK_FALSE Error during reading parts of POST body from client + * JK_TRUE JK_TRUE All other cases */ static int ajp_send_request(jk_endpoint_t *e, jk_ws_service_t *s, @@ -1464,7 +1477,7 @@ return JK_AJP13_ERROR; } r->http_response_status = res.status; - rc = is_http_status_fail(ae->worker, res.status); + rc = is_http_status_fail(ae->worker, res.status); if (rc > 0) { JK_TRACE_EXIT(l); return JK_STATUS_FATAL_ERROR; @@ -1543,7 +1556,7 @@ } jk_log(l, JK_LOG_INFO, - "Reding from client aborted or client network problems"); + "Reading from client aborted or client network problems"); JK_TRACE_EXIT(l); return JK_CLIENT_RD_ERROR; @@ -1603,6 +1616,26 @@ * ajp13/ajp14 is async but handling read/send this way prevent nice recovery * In fact if tomcat link is broken during upload (browser -> apache -> tomcat) * we'll loose data and we'll have to abort the whole request. + * + * Return values of ajp_get_reply() function: + * return value op->recoverable reason + * JK_REPLY_TIMEOUT ?recovery_options Reply timeout while waiting for response packet + * JK_FALSE ?recovery_options Error during ajp_connection_tcp_get_message() + * Communication error or wrong packet content while reading from backend. + * JK_STATUS_ERROR mostly JK_TRUE ajp_process_callback() returns JK_STATUS_ERROR + * Recoverable, if callback didn't return with a JK_HAS_RESPONSE before. + * JK_HAS_RESPONSE: parts of the post buffer are consumed. + * JK_STATUS_FATAL_ERROR mostly JK_TRUE ajp_process_callback() returns JK_STATUS_FATAL_ERROR + * Recoverable, if callback didn't return with a JK_HAS_RESPONSE before. + * JK_HAS_RESPONSE: parts of the post buffer are consumed. + * JK_SERVER_ERROR ? ajp_process_callback() returns JK_AJP13_ERROR + * JK_AJP13_ERROR: protocol error. + * JK_CLIENT_RD_ERROR ? ajp_process_callback() returns JK_CLIENT_RD_ERROR + * JK_CLIENT_RD_ERROR: could not read post from client. + * JK_CLIENT_WR_ERROR ? ajp_process_callback() returns JK_CLIENT_WR_ERROR + * JK_CLIENT_WR_ERROR: could not write back result to client + * JK_TRUE ? ajp_process_callback() returns JK_AJP13_END_RESPONSE + * JK_FALSE ? Other unhandled cases (unknown return codes) */ static int ajp_get_reply(jk_endpoint_t *e, jk_ws_service_t *s, @@ -1629,26 +1662,25 @@ if (headeratclient == JK_FALSE) { if (p->worker->recovery_opts & RECOVER_ABORT_IF_TCGETREQUEST) op->recoverable = JK_FALSE; + /* + * We revert back to recoverable, if recovery_opts allow it for GET or HEAD + */ + if (op->recoverable == JK_FALSE) { + if (p->worker->recovery_opts & RECOVER_ALWAYS_HTTP_HEAD) { + if (!strcmp(s->method, "HEAD")) + op->recoverable = JK_TRUE; + } + else if (p->worker->recovery_opts & RECOVER_ALWAYS_HTTP_GET) { + if (!strcmp(s->method, "GET")) + op->recoverable = JK_TRUE; + } + } } else { if (p->worker->recovery_opts & RECOVER_ABORT_IF_TCSENDHEADER) op->recoverable = JK_FALSE; } - /* - * We revert back to recoverable, if recovery_opts allow it for GET or HEAD - */ - if (op->recoverable == JK_FALSE) { - if (p->worker->recovery_opts & RECOVER_ALWAYS_HTTP_HEAD) { - if (!strcmp(s->method, "HEAD")) - op->recoverable = JK_TRUE; - } - else if (p->worker->recovery_opts & RECOVER_ALWAYS_HTTP_GET) { - if (!strcmp(s->method, "GET")) - op->recoverable = JK_TRUE; - } - } - JK_TRACE_EXIT(l); return JK_REPLY_TIMEOUT; } @@ -1786,15 +1818,6 @@ JK_TRACE_EXIT(l); return JK_SERVER_ERROR; } - else if (JK_FATAL_ERROR == rc) { - /* - * we won't be able to gracefully recover from this so - * set recoverable to false and get out. - */ - op->recoverable = JK_FALSE; - JK_TRACE_EXIT(l); - return JK_FALSE; - } else if (JK_CLIENT_RD_ERROR == rc) { /* * Client has stop sending to us, so get out. @@ -1811,18 +1834,29 @@ JK_TRACE_EXIT(l); return JK_CLIENT_WR_ERROR; } - else if (JK_SERVER_ERROR == rc) { + else if (JK_INTERNAL_ERROR == rc) { /* - * Tomcat has stop talking to us, so get out. - * Loadbalancer if present will decide if - * failover is possible. + * Internal error, like memory allocation or invalid packet lengths. */ JK_TRACE_EXIT(l); return JK_SERVER_ERROR; } + else if (JK_AJP13_NO_RESPONSE == rc) { + /* + * This is fine, loop again, more data to send. + */ + } else if (rc < 0) { + /* + * XXX: We should disable reuse in this case to. + * Maybe use another return value and let service() do the disable reuse. + */ + op->recoverable = JK_FALSE; + jk_log(l, JK_LOG_ERROR, + "(%s) Callback returns with unknown value %d", + p->worker->name, rc); JK_TRACE_EXIT(l); - return (JK_FALSE); /* XXX error */ + return JK_FALSE; } } /* XXX: Not reached? */ @@ -1836,6 +1870,28 @@ * * We serve here the request, using AJP13/AJP14 (e->proto) * + * Return values of service() method for ajp13/ajp14 worker: + * return value is_error op->recoverable reason + * JK_FALSE JK_HTTP_SERVER_ERROR UNDEF Invalid Parameters (null values) + * JK_SERVER_ERROR JK_HTTP_OK UNDEF Error during initializing empty request, response or post body objects + * JK_CLIENT_ERROR JK_REQUEST_TOO_LARGE JK_TRUE Request doesn't fit into buffer (error during ajp_marshal_into_msgb()) + * JK_CLIENT_ERROR JK_HTTP_BAD_REQUEST JK_FALSE Error during reading parts of POST body from client + * JK_SERVER_ERROR JK_HTTP_SERVER_ERROR JK_FALSE If ajp_send_request() returns JK_TRUE but !op->recoverable. + * This should never happen. + * JK_CLIENT_ERROR JK_HTTP_BAD_REQUEST ? ajp_get_reply() returns JK_CLIENT_RD_ERROR + * JK_CLIENT_ERROR JK_HTTP_OK ? ajp_get_reply() returns JK_CLIENT_WR_ERROR + * JK_SERVER_ERROR JK_HTTP_SERVER_ERROR ? ajp_get_reply() returns JK_SERVER_ERROR + * JK_SERVER_ERROR: protocol error or internal error + * JK_REPLY_TIMEOUT JK_HTTP_GATEWAY_TIME_OUT JK_FALSE ajp_get_reply() returns JK_REPLY_TIMEOUT + * Only if !op->recoverable + * JK_FALSE JK_HTTP_GATEWAY_TIME_OUT JK_FALSE ajp_get_reply() returns something else + * Only if !op->recoverable + * JK_REPLY_TIMEOUT JK_HTTP_GATEWAY_TIME_OUT JK_TRUE ajp_get_reply() returns JK_REPLY_TIMEOUT + * Only if op->recoverable and no more ajp13/ajp14 direct retries + * JK_STATUS_ERROR JK_STATUS_ERROR JK_TRUE ajp_get_reply() returns JK_STATUS_ERROR + * Only if op->recoverable and no more ajp13/ajp14 direct retries + * JK_FALSE JK_HTTP_SERVER_BUSY JK_TRUE ajp_get_reply() returns JK_FALSE or JK_STATUS_FATAL_ERROR + * Only if op->recoverable and no more ajp13/ajp14 direct retries */ static int JK_METHOD ajp_service(jk_endpoint_t *e, jk_ws_service_t *s, @@ -1948,20 +2004,39 @@ * to next valid tomcat. */ err = ajp_send_request(e, s, l, p, op); - if (err == JK_TRUE) { - - /* If we have an unrecoverable error, it's probably because - * the sender (browser) stopped sending data before the end - * (certainly in a big post) + if (err == JK_CLIENT_RD_ERROR) { + *is_error = JK_HTTP_BAD_REQUEST; + if (p->worker->recovery_opts & RECOVER_ABORT_IF_CLIENTERROR) { + /* Mark the endpoint for shutdown */ + p->reuse = JK_FALSE; + } + jk_log(l, JK_LOG_INFO, + "(%s) sending request to tomcat failed, " + "because of client read error " + "without recovery in send loop attempt=%d", + p->worker->name, i); + JK_TRACE_EXIT(l); + return JK_CLIENT_ERROR; + } + else if (err == JK_FALSE) { + jk_log(l, JK_LOG_INFO, + "(%s) sending request to tomcat failed, " + "recoverable operation attempt=%d", + p->worker->name, i); + } + else if (err == JK_TRUE) { + /* XXX: this should never happen: + * ajp_send_request() never returns JK_TRUE if !op->recoverable. */ if (!op->recoverable) { *is_error = JK_HTTP_SERVER_ERROR; jk_log(l, JK_LOG_ERROR, "(%s) sending request to tomcat failed " + "because of an unknown reason " "without recovery in send loop %d", p->worker->name, i); JK_TRACE_EXIT(l); - return JK_FALSE; + return JK_SERVER_ERROR; } /* Up to there we can recover */ @@ -2032,7 +2107,6 @@ * operation is no more recoverable */ if (!op->recoverable) { - *is_error = JK_HTTP_BAD_GATEWAY; jk_log(l, JK_LOG_ERROR, "(%s) receiving reply from tomcat failed " "without recovery in send loop attempt=%d", @@ -2042,6 +2116,7 @@ JK_TRACE_EXIT(l); return JK_REPLY_TIMEOUT; } + *is_error = JK_HTTP_BAD_GATEWAY; JK_TRACE_EXIT(l); return JK_FALSE; } @@ -2055,39 +2130,19 @@ } } } - if (err == JK_CLIENT_RD_ERROR) { - *is_error = JK_HTTP_BAD_REQUEST; - if (p->worker->recovery_opts & RECOVER_ABORT_IF_CLIENTERROR) { - /* Mark the endpoint for shutdown */ - p->reuse = JK_FALSE; - } - jk_log(l, JK_LOG_INFO, - "(%s) sending request to tomcat failed, " - "because of client read error " - "without recovery in send loop attempt=%d", - p->worker->name, i); - JK_TRACE_EXIT(l); - return JK_CLIENT_ERROR; - } - else if (err == JK_CLIENT_WR_ERROR) { - *is_error = JK_HTTP_OK; - if (p->worker->recovery_opts & RECOVER_ABORT_IF_CLIENTERROR) { - /* Mark the endpoint for shutdown */ - p->reuse = JK_FALSE; - } - jk_log(l, JK_LOG_INFO, - "(%s) sending request to tomcat failed, " - "because of client write error " - "without recovery in send loop attempt=%d", - p->worker->name, i); - JK_TRACE_EXIT(l); - return JK_CLIENT_ERROR; - } else { - jk_log(l, JK_LOG_INFO, - "(%s) sending request to tomcat failed, " - "recoverable operation attempt=%d", - p->worker->name, i + 1); + /* + * This should never happen. + * XXX: What is a good return value? + * XXX: What is a good is_error value? + * XXX: Do we need to set recoverable and/or reuse to false? + */ + op->recoverable = JK_FALSE; + jk_log(l, JK_LOG_ERROR, + "(%s) sending request to tomcat failed with unknown value %d", + p->worker->name, err); + JK_TRACE_EXIT(l); + return JK_FALSE; } /* Get another connection from the pool and try again. * Note: All sockets are probably closed already. Modified: tomcat/connectors/trunk/jk/native/common/jk_jni_worker.c URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_jni_worker.c?rev=572181&r1=572180&r2=572181&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/native/common/jk_jni_worker.c (original) +++ tomcat/connectors/trunk/jk/native/common/jk_jni_worker.c Sun Sep 2 15:50:40 2007 @@ -248,6 +248,14 @@ } #endif +/* + * Return values of service() method for jni worker: + * return value is_error reason + * JK_FALSE JK_HTTP_SERVER_ERROR Invalid parameters (null values) + * Error during attach to the JNI backend + * Error during JNI call + * JK_TRUE JK_HTTP_OK All other cases + */ static int JK_METHOD service(jk_endpoint_t *e, jk_ws_service_t *s, jk_logger_t *l, int *is_error) Modified: tomcat/connectors/trunk/jk/native/common/jk_status.c URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_status.c?rev=572181&r1=572180&r2=572181&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/native/common/jk_status.c (original) +++ tomcat/connectors/trunk/jk/native/common/jk_status.c Sun Sep 2 15:50:40 2007 @@ -2969,6 +2969,12 @@ return JK_FALSE; } +/* + * Return values of service() method for status worker: + * return value is_error reason + * JK_FALSE JK_HTTP_SERVER_ERROR Invalid parameters (null values) + * JK_TRUE JK_HTTP_OK All other cases + */ static int JK_METHOD service(jk_endpoint_t *e, jk_ws_service_t *s, jk_logger_t *l, int *is_error) --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]