Re: svn commit: r1887176 - /httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c

[jean-frederic clere]

On 08/03/2021 08:38, Ruediger Pluem wrote:



On 3/4/21 3:00 PM, jfcl...@apache.org wrote:

Author: jfclere
Date: Thu Mar  4 14:00:45 2021
New Revision: 1887176

URL: http://svn.apache.org/viewvc?rev=1887176=rev
Log:
Add balancer_manage() to allow external module to fill workers for balancers.

Modified:
 httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1887176=1887175=1887176=diff
==
--- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Thu Mar  4 14:00:45 
2021
@@ -1376,6 +1376,42 @@ static int balancer_process_balancer_wor
  }
  
  /*

+ * Process a request for balancer or worker management from another module
+ */
+static int balancer_manage(request_rec *r, apr_table_t *params)
+{
+void *sconf;
+proxy_server_conf *conf;
+proxy_balancer *bsel = NULL;
+proxy_worker *wsel = NULL;
+const char *name;
+sconf = r->server->module_config;
+conf = (proxy_server_conf *) ap_get_module_config(sconf, _module);
+
+/* Process the parameters */
+if ((name = apr_table_get(params, "b"))) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
+  "balancer: %s", name);
+bsel = ap_proxy_get_balancer(r->pool, conf,
+apr_pstrcat(r->pool, BALANCER_PREFIX, name, NULL), 0);
+}
+
+if ((name = apr_table_get(params, "w"))) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
+  "worker: %s", name);
+wsel = ap_proxy_get_worker(r->pool, bsel, conf, name);
+}
+if (bsel) {
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
+  "balancer: %s",  bsel->s->name);
+return(balancer_process_balancer_worker(r, conf, bsel, wsel, params));
+}
+ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage failed: "
+  "No balancer!");
+return HTTP_BAD_REQUEST;
+}
+
+/*
   * builds the page and links to configure via HTLM or XML.
   */
  static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
@@ -2024,6 +2060,7 @@ static void ap_proxy_balancer_register_h
  static const char *const aszPred[] = { "mpm_winnt.c", 
"mod_slotmem_shm.c", NULL};
  static const char *const aszPred2[] = { "mod_proxy.c", NULL};
   /* manager handler */
+ap_register_provider(p, "balancer", "manager", "0", _manage);


Wouldn't it be better to create a structure in mod_proxy.h that currently has 
only one field namely a function pointer
of int balancer_manage (request_rec *, apr_table_t *). This would declare this 
as a public API and would make
it possible to extend it later if needed.
If the API is planned to be limited to just this single function wouldn't an 
optional function serve us better here?


My plans are to extent balancer_manage() by supporting more parameters, 
but the approach to create a structure of function pointers is probably 
more flexible, I will prepare a new proposal this week.




Regards

Rüdiger




--
Cheers

Jean-Frederic

Re: svn commit: r1887176 - /httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c

[Ruediger Pluem]

On 3/4/21 3:00 PM, jfcl...@apache.org wrote:
> Author: jfclere
> Date: Thu Mar  4 14:00:45 2021
> New Revision: 1887176
> 
> URL: http://svn.apache.org/viewvc?rev=1887176=rev
> Log:
> Add balancer_manage() to allow external module to fill workers for balancers.
> 
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> 
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1887176=1887175=1887176=diff
> ==
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Thu Mar  4 14:00:45 
> 2021
> @@ -1376,6 +1376,42 @@ static int balancer_process_balancer_wor
>  }
>  
>  /*
> + * Process a request for balancer or worker management from another module
> + */
> +static int balancer_manage(request_rec *r, apr_table_t *params)
> +{
> +void *sconf;
> +proxy_server_conf *conf;
> +proxy_balancer *bsel = NULL;
> +proxy_worker *wsel = NULL;
> +const char *name;
> +sconf = r->server->module_config;
> +conf = (proxy_server_conf *) ap_get_module_config(sconf, _module);
> +
> +/* Process the parameters */
> +if ((name = apr_table_get(params, "b"))) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
> +  "balancer: %s", name);
> +bsel = ap_proxy_get_balancer(r->pool, conf,
> +apr_pstrcat(r->pool, BALANCER_PREFIX, name, NULL), 0);
> +}
> +
> +if ((name = apr_table_get(params, "w"))) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
> +  "worker: %s", name);
> +wsel = ap_proxy_get_worker(r->pool, bsel, conf, name);
> +}
> +if (bsel) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage "
> +  "balancer: %s",  bsel->s->name);
> +return(balancer_process_balancer_worker(r, conf, bsel, wsel, 
> params));
> +}
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "balancer_manage failed: "
> +  "No balancer!");
> +return HTTP_BAD_REQUEST;
> +}
> +
> +/*
>   * builds the page and links to configure via HTLM or XML.
>   */
>  static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
> @@ -2024,6 +2060,7 @@ static void ap_proxy_balancer_register_h
>  static const char *const aszPred[] = { "mpm_winnt.c", 
> "mod_slotmem_shm.c", NULL};
>  static const char *const aszPred2[] = { "mod_proxy.c", NULL};
>   /* manager handler */
> +ap_register_provider(p, "balancer", "manager", "0", _manage);

Wouldn't it be better to create a structure in mod_proxy.h that currently has 
only one field namely a function pointer
of int balancer_manage (request_rec *, apr_table_t *). This would declare this 
as a public API and would make
it possible to extend it later if needed.
If the API is planned to be limited to just this single function wouldn't an 
optional function serve us better here?

Regards

Rüdiger