You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Graham Leggett <mi...@sharp.fm> on 2018/02/17 23:11:10 UTC
[Patch] proof of concept - teaching hooks to yield where sensible
Hi all,
As an extension to the idea of filters being async and being able to yield and break up their work into small chunks, I am keen to extend that idea to selected hooks.
The patch below is a proof of concept, showing what it might look like if the pre_connection and process_connection hooks were able to return the code AGAIN. This means “please call me again”.
In implementing this, we would start at the outermost hook, and then work inwards. We would also need to change these void functions to return ints, so that AGAIN could bubble upwards. Eventually the handler hook will be able to return AGAIN.
Any module that doesn’t yield just behaves as modules do now.
Does this make sense?
Regards,
Graham
--
Index: include/http_connection.h
===================================================================
--- include/http_connection.h (revision 1824594)
+++ include/http_connection.h (working copy)
@@ -105,11 +105,14 @@
* This hook gives protocol modules an opportunity to set everything up
* before calling the protocol handler. All pre-connection hooks are
* run until one returns something other than ok or decline
+ *
+ * The pre connection hook may trigger a break in processing and ask to
+ * be called again by returning AGAIN.
* @param c The connection on which the request has been received.
* @param csd The mechanism on which this connection is to be read.
* Most times this will be a socket, but it is up to the module
* that accepts the request to determine the exact type.
- * @return OK or DECLINED
+ * @return OK, DECLINED or AGAIN
*/
AP_DECLARE_HOOK(int,pre_connection,(conn_rec *c, void *csd))
@@ -118,8 +121,11 @@
* established, the protocol module must read and serve the request. This
* function does that for each protocol module. The first protocol module
* to handle the request is the last module run.
+ *
+ * Protocol modules may trigger a break in processing and ask to be called
+ * again by returning AGAIN.
* @param c The connection on which the request has been received.
- * @return OK or DECLINED
+ * @return OK, DECLINED or AGAIN
*/
AP_DECLARE_HOOK(int,process_connection,(conn_rec *c))
Index: include/httpd.h
===================================================================
--- include/httpd.h (revision 1824594)
+++ include/httpd.h (working copy)
@@ -460,6 +460,8 @@
*/
#define SUSPENDED -3 /**< Module will handle the remainder of the request.
* The core will never invoke the request again, */
+#define AGAIN -4 /**< Module wants to be called again.
+ */
/** Returned by the bottom-most filter if no data was written.
* @see ap_pass_brigade(). */
Index: server/connection.c
===================================================================
--- server/connection.c (revision 1824594)
+++ server/connection.c (working copy)
@@ -29,6 +29,7 @@
#include "scoreboard.h"
#include "http_log.h"
#include "util_filter.h"
+#include "core.h"
APR_HOOK_STRUCT(
APR_HOOK_LINK(create_connection)
@@ -207,15 +208,38 @@
AP_CORE_DECLARE(void) ap_process_connection(conn_rec *c, void *csd)
{
+ conn_config_t *conn_config = ap_get_core_module_config(c->conn_config);
int rc;
+
+ switch (AP_CORE_DEFAULT(conn_config, milestone, CONN_MILESTONE_START)) {
+ case CONN_MILESTONE_START:
+
ap_update_vhost_given_ip(c);
+ case CONN_MILESTONE_PRE_CONNECTION:
+ conn_config->milestone = CONN_MILESTONE_START;
rc = ap_run_pre_connection(c, csd);
+ if (rc == AGAIN) {
+ conn_config->milestone = CONN_MILESTONE_PRE_CONNECTION;
+ return;
+ }
+
if (rc != OK && rc != DONE) {
c->aborted = 1;
}
if (!c->aborted) {
- ap_run_process_connection(c);
+
+ case CONN_MILESTONE_PROCESS_CONNECTION:
+ conn_config->milestone = CONN_MILESTONE_START;
+ rc = ap_run_process_connection(c);
+ if (rc == AGAIN) {
+ conn_config->milestone = CONN_MILESTONE_PROCESS_CONNECTION;
+ return;
+ }
+
}
+
+ }
}
+
Index: server/core.h
===================================================================
--- server/core.h (revision 1824635)
+++ server/core.h (working copy)
@@ -25,6 +25,13 @@
#ifndef CORE_H
#define CORE_H
+/** The connection milestones in the core */
+typedef enum {
+ CONN_MILESTONE_START,
+ CONN_MILESTONE_PRE_CONNECTION,
+ CONN_MILESTONE_PROCESS_CONNECTION
+} conn_milestone_e;
+
/**
* @brief A structure to contain connection state information
*/
@@ -31,6 +38,8 @@
typedef struct conn_config_t {
/** Socket belonging to the connection */
apr_socket_t *socket;
+ /** Connection milestones */
+ conn_milestone_e milestone;
} conn_config_t;
Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c (revision 1824594)
+++ server/mpm/event/event.c (working copy)
@@ -219,6 +219,14 @@
*/
static apr_pollset_t *event_pollset;
+typedef enum event_milestone_e event_milestone_e;
+
+enum event_milestone_e {
+ EVENT_MILESTONE_START = 0,
+ EVENT_MILESTONE_PRE_CONNECTION,
+ EVENT_MILESTONE_PROCESS_CONNECTION
+};
+
typedef struct event_conn_state_t event_conn_state_t;
/*
@@ -245,6 +253,8 @@
* hooks)
*/
int suspended;
+ /** current milestone */
+ event_milestone_e milestone;
/** memory pool to allocate from */
apr_pool_t *p;
/** bucket allocator */
@@ -996,6 +1006,9 @@
apr_status_t rv;
int rc = OK;
+ switch (AP_CORE_DEFAULT(cs, milestone, EVENT_MILESTONE_START)) {
+ case EVENT_MILESTONE_START:
+
if (cs == NULL) { /* This is a new connection */
listener_poll_type *pt = apr_pcalloc(p, sizeof(*pt));
cs = apr_pcalloc(p, sizeof(event_conn_state_t));
@@ -1028,7 +1041,14 @@
ap_update_vhost_given_ip(c);
+ case EVENT_MILESTONE_PRE_CONNECTION:
+ cs->milestone = EVENT_MILESTONE_START;
rc = ap_run_pre_connection(c, sock);
+ if (rc == AGAIN) {
+ cs->milestone = EVENT_MILESTONE_PRE_CONNECTION;
+ return;
+ }
+
if (rc != OK && rc != DONE) {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(00469)
"process_socket: connection aborted");
@@ -1083,7 +1103,15 @@
if (clogging) {
apr_atomic_inc32(&clogged_count);
}
+
+ case EVENT_MILESTONE_PROCESS_CONNECTION:
+ cs->milestone = EVENT_MILESTONE_START;
rc = ap_run_process_connection(c);
+ if (rc == AGAIN) {
+ cs->milestone = EVENT_MILESTONE_PROCESS_CONNECTION;
+ return;
+ }
+
if (clogging) {
apr_atomic_dec32(&clogged_count);
}
@@ -1095,6 +1123,7 @@
}
}
}
+
/*
* The process_connection hooks above should set the connection state
* appropriately upon return, for event MPM to either:
@@ -1245,6 +1274,8 @@
cs->pub.state == CONN_STATE_LINGER_SHORT)) {
process_lingering_close(cs);
}
+
+ }
}
/* Put a SUSPENDED connection back into a queue. */
Regards,
Graham
—
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Jim Jagielski <ji...@jaguNET.com>.
Would it be possible to leverage something like libdill (http://libdill.org) for that?
> On Feb 19, 2018, at 5:24 AM, Graham Leggett <mi...@sharp.fm> wrote:
>>
>
> This is the problem I want to solve - I want to be able to run multiple connections, and allow them to yield to each other.
>
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 19.02.2018 um 11:24 schrieb Graham Leggett <mi...@sharp.fm>:
>
> On 19 Feb 2018, at 11:14 AM, Stefan Eissing <st...@greenbytes.de> wrote:
>
>> If I understand your gist correctly, this would allow HTTP/2 processing to return to the main (async) event loop more often. Which would be great.
>>
>> In the case of HTTP/2, it would be even more cool, to trigger the (re-)processing of an AGAIN connection from another thread. The use
>> case is: H2 main look started request, awaits response HEADERS/DATA *or* incoming data from client.
>>
>> now: timed wait on a condition with read checks on the main connection at dynamic intervals
>> then: return AGAIN (READ mode) to event look, new HEADERS/DATA from request triggers re-process_connection.
>
> This is the problem I want to solve - I want to be able to run multiple connections, and allow them to yield to each other.
>
> I want to give our hooks the option to bite off and process data in chunks they’re in control of. Right now, you call the handler hook and it’s a one shot deal - the handler must finish whatever it wants to do, and only return once done.
>
> Yes, in most cases our handlers generate data and pass them to the filter chain, which then handles the async data flow, but it would be nice if they weren’t forced to do that.
+1
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Graham Leggett <mi...@sharp.fm>.
On 19 Feb 2018, at 11:14 AM, Stefan Eissing <st...@greenbytes.de> wrote:
> If I understand your gist correctly, this would allow HTTP/2 processing to return to the main (async) event loop more often. Which would be great.
>
> In the case of HTTP/2, it would be even more cool, to trigger the (re-)processing of an AGAIN connection from another thread. The use
> case is: H2 main look started request, awaits response HEADERS/DATA *or* incoming data from client.
>
> now: timed wait on a condition with read checks on the main connection at dynamic intervals
> then: return AGAIN (READ mode) to event look, new HEADERS/DATA from request triggers re-process_connection.
This is the problem I want to solve - I want to be able to run multiple connections, and allow them to yield to each other.
I want to give our hooks the option to bite off and process data in chunks they’re in control of. Right now, you call the handler hook and it’s a one shot deal - the handler must finish whatever it wants to do, and only return once done.
Yes, in most cases our handlers generate data and pass them to the filter chain, which then handles the async data flow, but it would be nice if they weren’t forced to do that.
Regards,
Graham
—
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Stefan Eissing <st...@greenbytes.de>.
If I understand your gist correctly, this would allow HTTP/2 processing to return to the main (async) event loop more often. Which would be great.
In the case of HTTP/2, it would be even more cool, to trigger the (re-)processing of an AGAIN connection from another thread. The use
case is: H2 main look started request, awaits response HEADERS/DATA *or* incoming data from client.
now: timed wait on a condition with read checks on the main connection at dynamic intervals
then: return AGAIN (READ mode) to event look, new HEADERS/DATA from request triggers re-process_connection.
-Stefan
> Am 18.02.2018 um 00:11 schrieb Graham Leggett <mi...@sharp.fm>:
>
> Hi all,
>
> As an extension to the idea of filters being async and being able to yield and break up their work into small chunks, I am keen to extend that idea to selected hooks.
>
> The patch below is a proof of concept, showing what it might look like if the pre_connection and process_connection hooks were able to return the code AGAIN. This means “please call me again”.
>
> In implementing this, we would start at the outermost hook, and then work inwards. We would also need to change these void functions to return ints, so that AGAIN could bubble upwards. Eventually the handler hook will be able to return AGAIN.
>
> Any module that doesn’t yield just behaves as modules do now.
>
> Does this make sense?
>
> Regards,
> Graham
> --
>
> Index: include/http_connection.h
> ===================================================================
> --- include/http_connection.h (revision 1824594)
> +++ include/http_connection.h (working copy)
> @@ -105,11 +105,14 @@
> * This hook gives protocol modules an opportunity to set everything up
> * before calling the protocol handler. All pre-connection hooks are
> * run until one returns something other than ok or decline
> + *
> + * The pre connection hook may trigger a break in processing and ask to
> + * be called again by returning AGAIN.
> * @param c The connection on which the request has been received.
> * @param csd The mechanism on which this connection is to be read.
> * Most times this will be a socket, but it is up to the module
> * that accepts the request to determine the exact type.
> - * @return OK or DECLINED
> + * @return OK, DECLINED or AGAIN
> */
> AP_DECLARE_HOOK(int,pre_connection,(conn_rec *c, void *csd))
>
> @@ -118,8 +121,11 @@
> * established, the protocol module must read and serve the request. This
> * function does that for each protocol module. The first protocol module
> * to handle the request is the last module run.
> + *
> + * Protocol modules may trigger a break in processing and ask to be called
> + * again by returning AGAIN.
> * @param c The connection on which the request has been received.
> - * @return OK or DECLINED
> + * @return OK, DECLINED or AGAIN
> */
> AP_DECLARE_HOOK(int,process_connection,(conn_rec *c))
>
> Index: include/httpd.h
> ===================================================================
> --- include/httpd.h (revision 1824594)
> +++ include/httpd.h (working copy)
> @@ -460,6 +460,8 @@
> */
> #define SUSPENDED -3 /**< Module will handle the remainder of the request.
> * The core will never invoke the request again, */
> +#define AGAIN -4 /**< Module wants to be called again.
> + */
>
> /** Returned by the bottom-most filter if no data was written.
> * @see ap_pass_brigade(). */
> Index: server/connection.c
> ===================================================================
> --- server/connection.c (revision 1824594)
> +++ server/connection.c (working copy)
> @@ -29,6 +29,7 @@
> #include "scoreboard.h"
> #include "http_log.h"
> #include "util_filter.h"
> +#include "core.h"
>
> APR_HOOK_STRUCT(
> APR_HOOK_LINK(create_connection)
> @@ -207,15 +208,38 @@
>
> AP_CORE_DECLARE(void) ap_process_connection(conn_rec *c, void *csd)
> {
> + conn_config_t *conn_config = ap_get_core_module_config(c->conn_config);
> int rc;
> +
> + switch (AP_CORE_DEFAULT(conn_config, milestone, CONN_MILESTONE_START)) {
> + case CONN_MILESTONE_START:
> +
> ap_update_vhost_given_ip(c);
>
> + case CONN_MILESTONE_PRE_CONNECTION:
> + conn_config->milestone = CONN_MILESTONE_START;
> rc = ap_run_pre_connection(c, csd);
> + if (rc == AGAIN) {
> + conn_config->milestone = CONN_MILESTONE_PRE_CONNECTION;
> + return;
> + }
> +
> if (rc != OK && rc != DONE) {
> c->aborted = 1;
> }
>
> if (!c->aborted) {
> - ap_run_process_connection(c);
> +
> + case CONN_MILESTONE_PROCESS_CONNECTION:
> + conn_config->milestone = CONN_MILESTONE_START;
> + rc = ap_run_process_connection(c);
> + if (rc == AGAIN) {
> + conn_config->milestone = CONN_MILESTONE_PROCESS_CONNECTION;
> + return;
> + }
> +
> }
> +
> + }
> }
> +
> Index: server/core.h
> ===================================================================
> --- server/core.h (revision 1824635)
> +++ server/core.h (working copy)
> @@ -25,6 +25,13 @@
> #ifndef CORE_H
> #define CORE_H
>
> +/** The connection milestones in the core */
> +typedef enum {
> + CONN_MILESTONE_START,
> + CONN_MILESTONE_PRE_CONNECTION,
> + CONN_MILESTONE_PROCESS_CONNECTION
> +} conn_milestone_e;
> +
> /**
> * @brief A structure to contain connection state information
> */
> @@ -31,6 +38,8 @@
> typedef struct conn_config_t {
> /** Socket belonging to the connection */
> apr_socket_t *socket;
> + /** Connection milestones */
> + conn_milestone_e milestone;
> } conn_config_t;
>
>
> Index: server/mpm/event/event.c
> ===================================================================
> --- server/mpm/event/event.c (revision 1824594)
> +++ server/mpm/event/event.c (working copy)
> @@ -219,6 +219,14 @@
> */
> static apr_pollset_t *event_pollset;
>
> +typedef enum event_milestone_e event_milestone_e;
> +
> +enum event_milestone_e {
> + EVENT_MILESTONE_START = 0,
> + EVENT_MILESTONE_PRE_CONNECTION,
> + EVENT_MILESTONE_PROCESS_CONNECTION
> +};
> +
> typedef struct event_conn_state_t event_conn_state_t;
>
> /*
> @@ -245,6 +253,8 @@
> * hooks)
> */
> int suspended;
> + /** current milestone */
> + event_milestone_e milestone;
> /** memory pool to allocate from */
> apr_pool_t *p;
> /** bucket allocator */
> @@ -996,6 +1006,9 @@
> apr_status_t rv;
> int rc = OK;
>
> + switch (AP_CORE_DEFAULT(cs, milestone, EVENT_MILESTONE_START)) {
> + case EVENT_MILESTONE_START:
> +
> if (cs == NULL) { /* This is a new connection */
> listener_poll_type *pt = apr_pcalloc(p, sizeof(*pt));
> cs = apr_pcalloc(p, sizeof(event_conn_state_t));
> @@ -1028,7 +1041,14 @@
>
> ap_update_vhost_given_ip(c);
>
> + case EVENT_MILESTONE_PRE_CONNECTION:
> + cs->milestone = EVENT_MILESTONE_START;
> rc = ap_run_pre_connection(c, sock);
> + if (rc == AGAIN) {
> + cs->milestone = EVENT_MILESTONE_PRE_CONNECTION;
> + return;
> + }
> +
> if (rc != OK && rc != DONE) {
> ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(00469)
> "process_socket: connection aborted");
> @@ -1083,7 +1103,15 @@
> if (clogging) {
> apr_atomic_inc32(&clogged_count);
> }
> +
> + case EVENT_MILESTONE_PROCESS_CONNECTION:
> + cs->milestone = EVENT_MILESTONE_START;
> rc = ap_run_process_connection(c);
> + if (rc == AGAIN) {
> + cs->milestone = EVENT_MILESTONE_PROCESS_CONNECTION;
> + return;
> + }
> +
> if (clogging) {
> apr_atomic_dec32(&clogged_count);
> }
> @@ -1095,6 +1123,7 @@
> }
> }
> }
> +
> /*
> * The process_connection hooks above should set the connection state
> * appropriately upon return, for event MPM to either:
> @@ -1245,6 +1274,8 @@
> cs->pub.state == CONN_STATE_LINGER_SHORT)) {
> process_lingering_close(cs);
> }
> +
> + }
> }
>
> /* Put a SUSPENDED connection back into a queue. */
>
>
> Regards,
> Graham
> —
>
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Graham Leggett <mi...@sharp.fm>.
On 19 Feb 2018, at 11:45 AM, Yann Ylavic <yl...@gmail.com> wrote:
> We already have some event's mechanism(s) be async at the handler
> layer (write completion, callbacks in trunk).
> The "common connection state/milestone" proposal looks interesting for
> compatibility (maybe add new CONN_STATEs instead of the similar
> milestone type?).
The milestones are completely independent of the connection state, they simply mean “in this module, in this function, we have successfully got this far, if a hook returned AGAIN, continue from here and do not repeat hooks that have completed successfully”.
> However the most challenging part seems to be in/output filters part,
> they currently handle EAGAIN like a real error (logging, brigade
> cleanup, ...) and usually don't expect to be called again.
This is a solved problem in trunk.
It turned out the network filter was async already, and had the ability to be called again. In the async filter patches I abstracted this behaviour out and taught other filters how to do this - most notably mod_ssl.
This problem doesn’t even need to be solved for this to work - if a module chooses not to return AGAIN, that’s completely fine.
> We can surely address all the cases upstream, what about third parties?
Third party modules just work like they have before, and the behaviour is the same as before - a slot is consumed until done.
Regards,
Graham
—
Re: [Patch] proof of concept - teaching hooks to yield where sensible
Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Feb 18, 2018 at 12:11 AM, Graham Leggett <mi...@sharp.fm> wrote:>
> As an extension to the idea of filters being async and being able to
> yield and break up their work into small chunks, I am keen to extend
> that idea to selected hooks.
>
> The patch below is a proof of concept, showing what it might look
> like if the pre_connection and process_connection hooks were able to
> return the code AGAIN. This means “please call me again”.
>
> In implementing this, we would start at the outermost hook, and then
> work inwards. We would also need to change these void functions to
> return ints, so that AGAIN could bubble upwards. Eventually the
> handler hook will be able to return AGAIN.
>
> Any module that doesn’t yield just behaves as modules do now.
We already have some event's mechanism(s) be async at the handler
layer (write completion, callbacks in trunk).
The "common connection state/milestone" proposal looks interesting for
compatibility (maybe add new CONN_STATEs instead of the similar
milestone type?).
However the most challenging part seems to be in/output filters part,
they currently handle EAGAIN like a real error (logging, brigade
cleanup, ...) and usually don't expect to be called again.
We can surely address all the cases upstream, what about third parties?
Regards,
Yann.