Re: [Patch] proof of concept - teaching hooks to yield where sensible

[Jim Jagielski]

Would it be possible to leverage something like libdill (http://libdill.org) 
for that?

> On Feb 19, 2018, at 5:24 AM, Graham Leggett  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

[Graham Leggett]

On 19 Feb 2018, at 11:45 AM, Yann Ylavic  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
—



smime.p7s
Description: S/MIME cryptographic signature

Re: [Patch] proof of concept - teaching hooks to yield where sensible

[Stefan Eissing]

> Am 19.02.2018 um 11:24 schrieb Graham Leggett :
> 
> On 19 Feb 2018, at 11:14 AM, Stefan Eissing  
> 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

[Graham Leggett]

On 19 Feb 2018, at 11:14 AM, Stefan Eissing  
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
—



smime.p7s
Description: S/MIME cryptographic signature

Re: [Patch] proof of concept - teaching hooks to yield where sensible

[Yann Ylavic]

On Sun, Feb 18, 2018 at 12:11 AM, Graham Leggett  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.

Re: [Patch] proof of concept - teaching hooks to yield where sensible

[Stefan Eissing]

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 :
> 
> 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;
> +