From cef12c796b55ec69bae5de18d4747dd7cdbc8386 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 26 Feb 2019 14:45:36 +0100 Subject: [PATCH] v1.13.0 -------------------------------------------------------------------------------- * mod_proxy_http2: simplifying implementation and reducing memory use by only running a single request at the time on a h2 backend connection. The previous implementation was just not good enough regarding flow control on the overall server and too complex compared to the performance benefits achieved. The goal with this change is to establish a solid base from which to optimize things again. * Fixes slave connection input filter to use proper return code on speculative read encountering an EOF. * Starting test_600 for mod_proxy_http2 use on server itself --- ChangeLog | 8 + configure.ac | 2 +- mod_http2/h2_ngn_shed.c | 48 +----- mod_http2/h2_proxy_session.c | 47 +---- mod_http2/h2_proxy_session.h | 3 - mod_http2/h2_version.h | 4 +- mod_http2/mod_proxy_http2.c | 322 +++++++---------------------------- 7 files changed, 72 insertions(+), 362 deletions(-) diff --git a/ChangeLog b/ChangeLog index 30b12348..97407541 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +v1.13.0 +-------------------------------------------------------------------------------- + * mod_proxy_http2: simplifying implementation and reducing memory use by only + running a single request at the time on a h2 backend connection. The previous + implementation was just not good enough regarding flow control on the overall + server and too complex compared to the performance benefits achieved. The goal + with this change is to establish a solid base from which to optimize things + again. * Fixes slave connection input filter to use proper return code on speculative read encountering an EOF. * Starting test_600 for mod_proxy_http2 use on server itself diff --git a/configure.ac b/configure.ac index 9eb2eaea..98c8814a 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ # AC_PREREQ([2.69]) -AC_INIT([mod_http2], [1.12.5], [stefan.eissing@greenbytes.de]) +AC_INIT([mod_http2], [1.13.0], [stefan.eissing@greenbytes.de]) LT_PREREQ([2.2.6]) LT_INIT() diff --git a/mod_http2/h2_ngn_shed.c b/mod_http2/h2_ngn_shed.c index 39582592..108e4c00 100644 --- a/mod_http2/h2_ngn_shed.c +++ b/mod_http2/h2_ngn_shed.c @@ -145,17 +145,6 @@ void h2_ngn_shed_abort(h2_ngn_shed *shed) shed->aborted = 1; } -static void ngn_add_task(h2_req_engine *ngn, h2_task *task, request_rec *r) -{ - h2_ngn_entry *entry = apr_pcalloc(task->pool, sizeof(*entry)); - APR_RING_ELEM_INIT(entry, link); - entry->task = task; - entry->r = r; - H2_REQ_ENTRIES_INSERT_TAIL(&ngn->entries, entry); - ngn->no_assigned++; -} - - apr_status_t h2_ngn_shed_push_request(h2_ngn_shed *shed, const char *ngn_type, request_rec *r, http2_req_engine_init *einit) @@ -167,42 +156,7 @@ apr_status_t h2_ngn_shed_push_request(h2_ngn_shed *shed, const char *ngn_type, ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, shed->c, "h2_ngn_shed(%ld): PUSHing request (task=%s)", shed->c->id, task->id); - if (task->request->serialize) { - /* Max compatibility, deny processing of this */ - return APR_EOF; - } - - if (task->assigned) { - --task->assigned->no_assigned; - --task->assigned->no_live; - task->assigned = NULL; - } - - if (task->engine) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, task->c, - "h2_ngn_shed(%ld): push task(%s) hosting engine %s " - "already with %d tasks", - shed->c->id, task->id, task->engine->id, - task->engine->no_assigned); - task->assigned = task->engine; - ngn_add_task(task->engine, task, r); - return APR_SUCCESS; - } - - ngn = apr_hash_get(shed->ngns, ngn_type, APR_HASH_KEY_STRING); - if (ngn && !ngn->shutdown) { - /* this task will be processed in another thread, - * freeze any I/O for the time being. */ - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, task->c, - "h2_ngn_shed(%ld): pushing request %s to %s", - shed->c->id, task->id, ngn->id); - if (!h2_task_has_thawed(task)) { - h2_task_freeze(task); - } - ngn_add_task(ngn, task, r); - return APR_SUCCESS; - } - + /* no existing engine or being shut down, start a new one */ if (einit) { apr_status_t status; diff --git a/mod_http2/h2_proxy_session.c b/mod_http2/h2_proxy_session.c index b024c8c2..961f5e40 100644 --- a/mod_http2/h2_proxy_session.c +++ b/mod_http2/h2_proxy_session.c @@ -429,12 +429,6 @@ static int stream_response_data(nghttp2_session *ngh2, uint8_t flags, stream_id, NGHTTP2_STREAM_CLOSED); return NGHTTP2_ERR_STREAM_CLOSING; } - if (stream->standalone) { - nghttp2_session_consume(ngh2, stream_id, len); - ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, stream->r, - "h2_proxy_session(%s): stream %d, win_update %d bytes", - session->id, stream_id, (int)len); - } return 0; } @@ -641,7 +635,7 @@ h2_proxy_session *h2_proxy_session_setup(const char *id, proxy_conn_rec *p_conn, nghttp2_option_new(&option); nghttp2_option_set_peer_max_concurrent_streams(option, 100); - nghttp2_option_set_no_auto_window_update(option, 1); + nghttp2_option_set_no_auto_window_update(option, 0); nghttp2_session_client_new2(&session->ngh2, cbs, session, option); @@ -1545,42 +1539,3 @@ typedef struct { int updated; } win_update_ctx; -static int win_update_iter(void *udata, void *val) -{ - win_update_ctx *ctx = udata; - h2_proxy_stream *stream = val; - - if (stream->r && stream->r->connection == ctx->c) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, ctx->session->c, - "h2_proxy_session(%s-%d): win_update %ld bytes", - ctx->session->id, (int)stream->id, (long)ctx->bytes); - nghttp2_session_consume(ctx->session->ngh2, stream->id, ctx->bytes); - ctx->updated = 1; - return 0; - } - return 1; -} - - -void h2_proxy_session_update_window(h2_proxy_session *session, - conn_rec *c, apr_off_t bytes) -{ - if (!h2_proxy_ihash_empty(session->streams)) { - win_update_ctx ctx; - ctx.session = session; - ctx.c = c; - ctx.bytes = bytes; - ctx.updated = 0; - h2_proxy_ihash_iter(session->streams, win_update_iter, &ctx); - - if (!ctx.updated) { - /* could not find the stream any more, possibly closed, update - * the connection window at least */ - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c, - "h2_proxy_session(%s): win_update conn %ld bytes", - session->id, (long)bytes); - nghttp2_session_consume_connection(session->ngh2, (size_t)bytes); - } - } -} - diff --git a/mod_http2/h2_proxy_session.h b/mod_http2/h2_proxy_session.h index ecebb615..1d0750b0 100644 --- a/mod_http2/h2_proxy_session.h +++ b/mod_http2/h2_proxy_session.h @@ -120,9 +120,6 @@ void h2_proxy_session_cancel_all(h2_proxy_session *s); void h2_proxy_session_cleanup(h2_proxy_session *s, h2_proxy_request_done *done); -void h2_proxy_session_update_window(h2_proxy_session *s, - conn_rec *c, apr_off_t bytes); - #define H2_PROXY_REQ_URL_NOTE "h2-proxy-req-url" #endif /* h2_proxy_session_h */ diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h index d4841715..ad1cf706 100644 --- a/mod_http2/h2_version.h +++ b/mod_http2/h2_version.h @@ -27,7 +27,7 @@ * @macro * Version number of the http2 module as c string */ -#define MOD_HTTP2_VERSION "1.12.5-git" +#define MOD_HTTP2_VERSION "1.13.0-git" /** * @macro @@ -35,7 +35,7 @@ * release. This is a 24 bit number with 8 bits for major number, 8 bits * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203. */ -#define MOD_HTTP2_VERSION_NUM 0x010c05 +#define MOD_HTTP2_VERSION_NUM 0x010d00 #endif /* mod_h2_h2_version_h */ diff --git a/mod_http2/mod_proxy_http2.c b/mod_http2/mod_proxy_http2.c index 15418ded..8479d7a4 100644 --- a/mod_http2/mod_proxy_http2.c +++ b/mod_http2/mod_proxy_http2.c @@ -57,10 +57,10 @@ static void (*req_engine_done)(h2_req_engine *engine, conn_rec *r_conn, apr_status_t status); typedef struct h2_proxy_ctx { + const char *id; conn_rec *master; conn_rec *owner; apr_pool_t *pool; - request_rec *rbase; server_rec *server; const char *proxy_func; char server_portstr[32]; @@ -68,19 +68,16 @@ typedef struct h2_proxy_ctx { proxy_worker *worker; proxy_server_conf *conf; - h2_req_engine *engine; - const char *engine_id; - const char *engine_type; - apr_pool_t *engine_pool; apr_size_t req_buffer_size; - h2_proxy_fifo *requests; int capacity; - unsigned standalone : 1; unsigned is_ssl : 1; unsigned flushall : 1; - apr_status_t r_status; /* status of our first request work */ + request_rec *r; /* the request processed in this ctx */ + apr_status_t r_status; /* status of request work */ + int r_done; /* request was processed, not necessarily successfully */ + int r_may_retry; /* request may be retried */ h2_proxy_session *session; /* current http2 session against backend */ } h2_proxy_ctx; @@ -206,45 +203,6 @@ static int proxy_http2_canon(request_rec *r, char *url) return OK; } -static void out_consumed(void *baton, conn_rec *c, apr_off_t bytes) -{ - h2_proxy_ctx *ctx = baton; - - if (ctx->session) { - h2_proxy_session_update_window(ctx->session, c, bytes); - } -} - -static apr_status_t proxy_engine_init(h2_req_engine *engine, - const char *id, - const char *type, - apr_pool_t *pool, - apr_size_t req_buffer_size, - request_rec *r, - http2_output_consumed **pconsumed, - void **pctx) -{ - h2_proxy_ctx *ctx = ap_get_module_config(r->connection->conn_config, - &proxy_http2_module); - if (!ctx) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(03368) - "h2_proxy_session, engine init, no ctx found"); - return APR_ENOTIMPL; - } - - ctx->pool = pool; - ctx->engine = engine; - ctx->engine_id = id; - ctx->engine_type = type; - ctx->engine_pool = pool; - ctx->req_buffer_size = req_buffer_size; - ctx->capacity = H2MIN(100, h2_proxy_fifo_capacity(ctx->requests)); - - *pconsumed = out_consumed; - *pctx = ctx; - return APR_SUCCESS; -} - static apr_status_t add_request(h2_proxy_session *session, request_rec *r) { h2_proxy_ctx *ctx = session->user_data; @@ -254,7 +212,7 @@ static apr_status_t add_request(h2_proxy_session *session, request_rec *r) url = apr_table_get(r->notes, H2_PROXY_REQ_URL_NOTE); apr_table_setn(r->notes, "proxy-source-port", apr_psprintf(r->pool, "%hu", ctx->p_conn->connection->local_addr->port)); - status = h2_proxy_session_submit(session, url, r, ctx->standalone); + status = h2_proxy_session_submit(session, url, r, 1); if (status != APR_SUCCESS) { ap_log_cerror(APLOG_MARK, APLOG_ERR, status, r->connection, APLOGNO(03351) "pass request body failed to %pI (%s) from %s (%s)", @@ -268,43 +226,15 @@ static apr_status_t add_request(h2_proxy_session *session, request_rec *r) static void request_done(h2_proxy_ctx *ctx, request_rec *r, apr_status_t status, int touched) { - const char *task_id = apr_table_get(r->connection->notes, H2_TASK_ID_NOTE); - - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, r->connection, - "h2_proxy_session(%s): request done %s, touched=%d", - ctx->engine_id, task_id, touched); - if (status != APR_SUCCESS) { - if (!touched) { - /* untouched request, need rescheduling */ - status = h2_proxy_fifo_push(ctx->requests, r); - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, - APLOGNO(03369) - "h2_proxy_session(%s): rescheduled request %s", - ctx->engine_id, task_id); - return; - } - else { - const char *uri; - uri = apr_uri_unparse(r->pool, &r->parsed_uri, 0); - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, - APLOGNO(03471) "h2_proxy_session(%s): request %s -> %s " - "not complete, cannot repeat", - ctx->engine_id, task_id, uri); - } - } - - if (r == ctx->rbase) { + if (r == ctx->r) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, r->connection, + "h2_proxy_session(%s): request done, touched=%d", + ctx->id, touched); + ctx->r_done = 1; + if (touched) ctx->r_may_retry = 0; ctx->r_status = ((status == APR_SUCCESS)? APR_SUCCESS : HTTP_SERVICE_UNAVAILABLE); } - - if (req_engine_done && ctx->engine) { - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, - APLOGNO(03370) - "h2_proxy_session(%s): finished request %s", - ctx->engine_id, task_id); - req_engine_done(ctx->engine, r->connection, status); - } } static void session_req_done(h2_proxy_session *session, request_rec *r, @@ -313,43 +243,15 @@ static void session_req_done(h2_proxy_session *session, request_rec *r, request_done(session->user_data, r, status, touched); } -static apr_status_t next_request(h2_proxy_ctx *ctx, int before_leave) -{ - if (h2_proxy_fifo_count(ctx->requests) > 0) { - return APR_SUCCESS; - } - else if (req_engine_pull && ctx->engine) { - apr_status_t status; - request_rec *r = NULL; - - status = req_engine_pull(ctx->engine, before_leave? - APR_BLOCK_READ: APR_NONBLOCK_READ, - ctx->capacity, &r); - if (status == APR_SUCCESS && r) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, ctx->owner, - "h2_proxy_engine(%s): pulled request (%s) %s", - ctx->engine_id, - before_leave? "before leave" : "regular", - r->the_request); - h2_proxy_fifo_push(ctx->requests, r); - } - return APR_STATUS_IS_EAGAIN(status)? APR_SUCCESS : status; - } - return APR_EOF; -} - -static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) { +static apr_status_t ctx_run(h2_proxy_ctx *ctx) { apr_status_t status = OK; int h2_front; - request_rec *r; /* Step Four: Send the Request in a new HTTP/2 stream and * loop until we got the response or encounter errors. */ - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->owner, - "eng(%s): setup session", ctx->engine_id); h2_front = is_h2? is_h2(ctx->owner) : 0; - ctx->session = h2_proxy_session_setup(ctx->engine_id, ctx->p_conn, ctx->conf, + ctx->session = h2_proxy_session_setup(ctx->id, ctx->p_conn, ctx->conf, h2_front, 30, h2_proxy_log2((int)ctx->req_buffer_size), session_req_done); @@ -360,45 +262,20 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) { } ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03373) - "eng(%s): run session %s", ctx->engine_id, ctx->session->id); + "eng(%s): run session %s", ctx->id, ctx->session->id); ctx->session->user_data = ctx; - while (1) { - if (ctx->master->aborted) { - status = APR_ECONNABORTED; - goto out; - } - - if (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) { - add_request(ctx->session, r); - } + ctx->r_done = 0; + add_request(ctx->session, ctx->r); + + while (!ctx->master->aborted && !ctx->r_done) { + status = h2_proxy_session_process(ctx->session); - - if (status == APR_SUCCESS) { - /* ongoing processing, check if we have room to handle more streams, - * maybe the remote side changed their limit */ - if (ctx->session->remote_max_concurrent > 0 - && ctx->session->remote_max_concurrent != ctx->capacity) { - ctx->capacity = H2MIN((int)ctx->session->remote_max_concurrent, - h2_proxy_fifo_capacity(ctx->requests)); - } - /* try to pull more request, if our capacity allows it */ - if (APR_ECONNABORTED == next_request(ctx, 0)) { - status = APR_ECONNABORTED; - goto out; - } - /* If we have no ongoing streams and nothing in our queue, we - * terminate processing and return to our caller. */ - if ((h2_proxy_fifo_count(ctx->requests) == 0) - && h2_proxy_ihash_empty(ctx->session->streams)) { - goto out; - } - } - else { + if (status != APR_SUCCESS) { /* Encountered an error during session processing */ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, APLOGNO(03375) "eng(%s): end of session %s", - ctx->engine_id, ctx->session->id); + ctx->id, ctx->session->id); /* Any open stream of that session needs to * a) be reopened on the new session iff safe to do so * b) reported as done (failed) otherwise @@ -409,12 +286,11 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) { } out: - if (APR_ECONNABORTED == status) { + if (ctx->master->aborted) { /* master connection gone */ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, - APLOGNO(03374) "eng(%s): master connection gone", ctx->engine_id); - /* give notice that we're leaving and cancel all ongoing streams. */ - next_request(ctx, 1); + APLOGNO(03374) "eng(%s): master connection gone", ctx->id); + /* cancel all ongoing requests */ h2_proxy_session_cancel_all(ctx->session); h2_proxy_session_process(ctx->session); if (!ctx->master->aborted) { @@ -427,49 +303,6 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) { return status; } -static apr_status_t push_request_somewhere(h2_proxy_ctx *ctx, request_rec *r) -{ - conn_rec *c = ctx->owner; - const char *engine_type, *hostname; - - hostname = (ctx->p_conn->ssl_hostname? - ctx->p_conn->ssl_hostname : ctx->p_conn->hostname); - engine_type = apr_psprintf(ctx->pool, "proxy_http2 %s%s", hostname, - ctx->server_portstr); - - if (c->master && req_engine_push && r && is_h2 && is_h2(c)) { - /* If we are have req_engine capabilities, push the handling of this - * request (e.g. slave connection) to a proxy_http2 engine which - * uses the same backend. We may be called to create an engine - * ourself. */ - if (req_engine_push(engine_type, r, proxy_engine_init) == APR_SUCCESS) { - if (ctx->engine == NULL) { - /* request has been assigned to an engine in another thread */ - return SUSPENDED; - } - } - } - - if (!ctx->engine) { - /* No engine was available or has been initialized, handle this - * request just by ourself. */ - ctx->engine_id = apr_psprintf(ctx->pool, "eng-proxy-%ld", c->id); - ctx->engine_type = engine_type; - ctx->engine_pool = ctx->pool; - ctx->req_buffer_size = (32*1024); - ctx->standalone = 1; - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "h2_proxy_http2(%ld): setup standalone engine for type %s", - c->id, engine_type); - } - else { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "H2: hosting engine %s", ctx->engine_id); - } - - return h2_proxy_fifo_push(ctx->requests, r); -} - static int proxy_http2_handler(request_rec *r, proxy_worker *worker, proxy_server_conf *conf, @@ -477,7 +310,7 @@ static int proxy_http2_handler(request_rec *r, const char *proxyname, apr_port_t proxyport) { - const char *proxy_func; + const char *proxy_func, *task_id; char *locurl = url, *u; apr_size_t slen; int is_ssl = 0; @@ -509,34 +342,35 @@ static int proxy_http2_handler(request_rec *r, default: return DECLINED; } + + task_id = apr_table_get(r->connection->notes, H2_TASK_ID_NOTE); ctx = apr_pcalloc(r->pool, sizeof(*ctx)); - ctx->master = r->connection->master? r->connection->master : r->connection; - ctx->owner = r->connection; - ctx->pool = r->pool; - ctx->rbase = r; - ctx->server = r->server; + ctx->master = r->connection->master? r->connection->master : r->connection; + ctx->id = task_id? task_id : apr_psprintf(r->pool, "%ld", (long)ctx->master->id); + ctx->owner = r->connection; + ctx->pool = r->pool; + ctx->server = r->server; ctx->proxy_func = proxy_func; - ctx->is_ssl = is_ssl; - ctx->worker = worker; - ctx->conf = conf; - ctx->flushall = apr_table_get(r->subprocess_env, "proxy-flushall")? 1 : 0; - ctx->r_status = HTTP_SERVICE_UNAVAILABLE; - - h2_proxy_fifo_set_create(&ctx->requests, ctx->pool, 100); + ctx->is_ssl = is_ssl; + ctx->worker = worker; + ctx->conf = conf; + ctx->flushall = apr_table_get(r->subprocess_env, "proxy-flushall")? 1 : 0; + ctx->req_buffer_size = (32*1024); + ctx->r = r; + ctx->r_status = HTTP_SERVICE_UNAVAILABLE; + ctx->r_done = 0; + ctx->r_may_retry = 1; ap_set_module_config(ctx->owner->conn_config, &proxy_http2_module, ctx); /* scheme says, this is for us. */ - apr_table_setn(ctx->rbase->notes, H2_PROXY_REQ_URL_NOTE, url); - ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->rbase, + apr_table_setn(ctx->r->notes, H2_PROXY_REQ_URL_NOTE, url); + ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->r, "H2: serving URL %s", url); run_connect: - if (ctx->master->aborted) { - ctx->r_status = APR_ECONNABORTED; - goto cleanup; - } + if (ctx->master->aborted) goto cleanup; /* Get a proxy_conn_rec from the worker, might be a new one, might * be one still open from another request, or it might fail if the @@ -551,7 +385,7 @@ static int proxy_http2_handler(request_rec *r, /* Step One: Determine the URL to connect to (might be a proxy), * initialize the backend accordingly and determine the server * port string we can expect in responses. */ - if ((status = ap_proxy_determine_connection(ctx->pool, ctx->rbase, conf, worker, + if ((status = ap_proxy_determine_connection(ctx->pool, ctx->r, conf, worker, ctx->p_conn, &uri, &locurl, proxyname, proxyport, ctx->server_portstr, @@ -559,17 +393,6 @@ static int proxy_http2_handler(request_rec *r, goto cleanup; } - /* If we are not already hosting an engine, try to push the request - * to an already existing engine or host a new engine here. */ - if (r && !ctx->engine) { - ctx->r_status = push_request_somewhere(ctx, r); - r = NULL; - if (ctx->r_status == SUSPENDED) { - /* request was pushed to another thread, leave processing here */ - goto cleanup; - } - } - /* Step Two: Make the Connection (or check that an already existing * socket is still usable). On success, we have a socket connected to * backend->hostname. */ @@ -578,19 +401,19 @@ static int proxy_http2_handler(request_rec *r, ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03352) "H2: failed to make connection to backend: %s", ctx->p_conn->hostname); - goto reconnect; + goto cleanup; } /* Step Three: Create conn_rec for the socket we have open now. */ if (!ctx->p_conn->connection) { - status = ap_proxy_connection_create_ex(ctx->proxy_func, - ctx->p_conn, ctx->rbase); + status = ap_proxy_connection_create_ex(ctx->proxy_func, ctx->p_conn, ctx->r); if (status != OK) { ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, APLOGNO(03353) "setup new connection: is_ssl=%d %s %s %s", ctx->p_conn->is_ssl, ctx->p_conn->ssl_hostname, locurl, ctx->p_conn->hostname); - goto reconnect; + ctx->r_status = status; + goto cleanup; } if (!ctx->p_conn->data && ctx->is_ssl) { @@ -600,7 +423,7 @@ static int proxy_http2_handler(request_rec *r, apr_table_setn(ctx->p_conn->connection->notes, "proxy-request-alpn-protos", "h2"); if (ctx->p_conn->ssl_hostname) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, ctx->owner, + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->owner, "set SNI to %s for (%s)", ctx->p_conn->ssl_hostname, ctx->p_conn->hostname); @@ -610,33 +433,11 @@ static int proxy_http2_handler(request_rec *r, } } -run_session: - if (ctx->owner->aborted) { - ctx->r_status = APR_ECONNABORTED; - goto cleanup; - } + if (ctx->master->aborted) goto cleanup; + status = ctx_run(ctx); - status = proxy_engine_run(ctx); - if (status == APR_SUCCESS) { - /* session and connection still ok */ - if (next_request(ctx, 1) == APR_SUCCESS) { - /* more requests, run again */ - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03376) - "run_session, again"); - goto run_session; - } - /* done */ - ctx->engine = NULL; - } - -reconnect: - if (ctx->master->aborted) { - ctx->r_status = APR_ECONNABORTED; - goto cleanup; - } - - if (next_request(ctx, 1) == APR_SUCCESS) { - /* Still more to do, tear down old conn and start over */ + if (ctx->r_status != APR_SUCCESS && ctx->r_may_retry && !ctx->master->aborted) { + /* Not successfully processed, but may retry, tear down old conn and start over */ if (ctx->p_conn) { ctx->p_conn->close = 1; #if AP_MODULE_MAGIC_AT_LEAST(20140207, 2) @@ -646,12 +447,12 @@ static int proxy_http2_handler(request_rec *r, ctx->p_conn = NULL; } ++reconnects; - if (reconnects < 5 && !ctx->master->aborted) { + if (reconnects < 5) { goto run_connect; } ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(10023) - "giving up after %d reconnects, %d requests todo", - reconnects, h2_proxy_fifo_count(ctx->requests)); + "giving up after %d reconnects, request-done=%d", + reconnects, ctx->r_done); } cleanup: @@ -661,17 +462,12 @@ static int proxy_http2_handler(request_rec *r, ctx->p_conn->close = 1; } #if AP_MODULE_MAGIC_AT_LEAST(20140207, 2) - proxy_run_detach_backend(ctx->rbase, ctx->p_conn); + proxy_run_detach_backend(ctx->r, ctx->p_conn); #endif ap_proxy_release_connection(ctx->proxy_func, ctx->p_conn, ctx->server); ctx->p_conn = NULL; } - /* Any requests we still have need to fail */ - while (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) { - request_done(ctx, r, HTTP_SERVICE_UNAVAILABLE, 1); - } - ap_set_module_config(ctx->owner->conn_config, &proxy_http2_module, NULL); ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, APLOGNO(03377) "leaving handler");