[PATCH] worker status change during creation of backend proxy connection

[Ruediger Pluem]

Hi,

I noticed that the status of a connection (or better for a worker score)
is changed back to SERVER_BUSY_READ from SERVER_BUSY_WRITE, if this connection
requires that mod_proxy creates a new connection to a backend. This does not 
happen
if mod_proxy can pick up a connection to this backend from the connection pool.
This is a hint to me that the behaviour in the situation where a new connection 
gets
created is not correct.

I see two problems with the current behaviour:

1. mod_status shows the "wrong" status.
2. It is hard for functions in the pre_connection hook to decide if they are 
called
   for an outgoing proxy connection or for an incoming client connection.
   For sure for some this does not matter, but for others it does.
   If the status of the connection would not be changed they could determine 
this
   by checking if the status is SERVER_BUSY_READ (client connection) or
   SERVER_BUSY_WRITE (proxy connection to backend).

The reason for the status change is the execution of the create_connection hook
in ap_proxy_connection_create in proxy_util.c and thus the execution of
core_create_conn in core.c.

So my idea is to save the scoreboard entry for this slot before calling the
create_connection hook and restore it afterwards. If this is is not a solution
for the problem I fear that some refactoring of the connection creation process 
in
mod_proxy is needed.

Below there is a patch which does what I described. As struct ap_sb_handle_t is
private data to scoreboard.c, I created a small function that allows me to get 
the
pointer to worker_score based on the scoreboard handle.
I am not totally happy with the name of this function 
(ap_get_scoreboard_worker_sbh),
so any proposals for a different name are welcome.

Comments / thoughts?


Regards

Rüdiger
Index: server/scoreboard.c
===================================================================
--- server/scoreboard.c»(Revision 292553)
+++ server/scoreboard.c»(Arbeitskopie)
@@ -476,6 +476,13 @@
     return &ap_scoreboard_image->servers[x][y];
 }
·
+AP_DECLARE(worker_score *) ap_get_scoreboard_worker_sbh(ap_sb_handle_t *sbh)
+{
+    if (!sbh)
+        return(NULL);
+    return ap_get_scoreboard_worker(sbh->child_num, sbh->thread_num);
+}
+
 AP_DECLARE(process_score *) ap_get_scoreboard_process(int x)
 {
     if ((x < 0) || (server_limit < x)) {
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c»·(Revision 292553)
+++ modules/proxy/proxy_util.c»·(Arbeitskopie)
@@ -1995,13 +1995,35 @@
 {
     apr_sockaddr_t *backend_addr = conn->addr;
     int rc;
+    /* Pointer to scoreboard entry of current worker */
+    worker_score *current_worker_score;
+    /* Saves scoreboard entry of current worker temporarily */
+    worker_score current_worker_score_save;
·
+    /*
+     * As ap_run_create_connection modifies the scoreboard entry for the
+     * current worker, save the whole scoreboard entry for this worker
+     * and restore it after ap_run_create_connection has been run.
+     * Apart from the fact that mod_status will display correct information
+     * again it makes it possible for functions in the
+     * ap_run_pre_connection to check whether they are called for
+     * a mod_proxy connection or for an incoming connection by just checking
+     * the status of the current worker (whether its SERVER_BUSY_READ
+     * or SERVER_BUSY_WRITE).
+     */
+    current_worker_score = ap_get_scoreboard_worker_sbh(c->sbh);
+    memcpy(&current_worker_score_save, current_worker_score,
+           sizeof(worker_score));
+
     /* The socket is now open, create a new backend server connection·
     *·
     */
     conn->connection = ap_run_create_connection(c->pool, s, conn->sock,
                                                 c->id, c->sbh,
                                                 c->bucket_alloc);
+    /* Restore scoreboard entry for current worker */
+    memcpy(current_worker_score, &current_worker_score_save,
+           sizeof(worker_score));
·
     if (!conn->connection) {
         /* the peer reset the connection already; ap_run_create_connection()·
Index: include/scoreboard.h
===================================================================
--- include/scoreboard.h»·······(Revision 292553)
+++ include/scoreboard.h»·······(Arbeitskopie)
@@ -190,6 +190,7 @@
 void ap_time_process_request(ap_sb_handle_t *sbh, int status);
·
 AP_DECLARE(worker_score *) ap_get_scoreboard_worker(int x, int y);
+AP_DECLARE(worker_score *) ap_get_scoreboard_worker_sbh(ap_sb_handle_t *sbh);
 AP_DECLARE(process_score *) ap_get_scoreboard_process(int x);
 AP_DECLARE(global_score *) ap_get_scoreboard_global(void);
 AP_DECLARE(lb_score *) ap_get_scoreboard_lb(int lb_num);