Re: svn commit: r1887415 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h mod_proxy_ajp.c mod_proxy_balancer.c mod_proxy_hcheck.c

[jean-frederic clere]

On 10/03/2021 12:24, Ruediger Pluem wrote:

On 3/10/21 11:36 AM, jfcl...@apache.org wrote:

Author: jfclere
Date: Wed Mar 10 10:36:46 2021
New Revision: 1887415

URL: http://svn.apache.org/viewvc?rev=1887415&view=rev
Log:
Add CPING to health check logic.

Modified:
     httpd/httpd/trunk/modules/proxy/mod_proxy.h
     httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
     httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
     httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c?rev=1887415&r1=1887414&r2=1887415&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c Wed Mar 10 10:36:46 2021

@@ -613,6 +615,39 @@ static int hc_get_backend(const char *pr
      return hc_determine_connection(ctx, hc, &(*backend)->addr, ptemp);
  }

+static apr_status_t hc_check_cping(baton_t *baton)

+{
+    int status;
+    sctx_t *ctx = baton->ctx;
+    proxy_worker *hc = baton->hc;
+    proxy_conn_rec *backend = NULL;
+    apr_pool_t *ptemp = baton->ptemp;
+    request_rec *r;
+    apr_interval_time_t timeout;
+
+    if (!ajp_handle_cping_cpong) {
+        return APR_ENOTIMPL;
+    }
+
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, baton->ctx->s, "HCCPING 
starting");
+    if ((status = hc_get_backend("HCCPING", &backend, hc, ctx, baton->ptemp)) 
!= OK) {
+        return backend_cleanup("HCCPING", backend, ctx->s, status);
+    }
+    if ((status = ap_proxy_connect_backend("HCCPING", backend, hc, ctx->s)) != 
OK) {
+        return backend_cleanup("HCCPING", backend, ctx->s, status);
+    }
+    r = create_request_rec(ptemp, ctx->s, baton->balancer, "CPING");
+    if ((status = ap_proxy_connection_create_ex("HCCPING", backend, r)) != OK) 
{
+        return backend_cleanup("HCCPING", backend, ctx->s, status);
+    }
+    set_request_connection(r, backend->connection);
+
+    timeout = apr_time_from_sec(10); /* 10 seconds */

I don't like hardcoded timeouts. How about hc->s->ping_timeout instead?
This would require to copy the setting from the worker if present in 
hc_get_hcworker where we should copy the timeout field as
well from my point of view. We could make a cascaded choice here then:

hc->s->ping_timeout, hc->s->conn_timeout, hc->s->timeout, 
apr_socket_timeout_get(backend->sock)

Oops I will fix that ASAP thanks, I committed too early :-(

+    status = ajp_handle_cping_cpong(backend->sock, r, timeout);
+    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, baton->ctx->s, "HCCPING done %d", 
status);
+    return backend_cleanup("HCCPING", backend, ctx->s, status);
+}
+
  static apr_status_t hc_check_tcp(baton_t *baton)
  {
      int status;

Regards

Rüdiger

--
Cheers

Jean-Frederic