From: Stefan Eissing Date: Fri, 4 Dec 2015 14:53:13 +0000 (+0000) Subject: max connection window size, reducing write frequency again X-Git-Tag: 2.5.0-alpha~2566 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=900edcb9dcdd281670581da0e1347d315d88bd5e;p=apache max connection window size, reducing write frequency again git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1717975 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 3206bfbaf6..fff30f166b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,12 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.0 + *) mod_http2: connection level window for flow control is set to protocol + maximum of 2GB-1, preventing window exhaustion when sending data on many + streams with higher cumulative window size. + Reducing write frequency unless push promises need to be flushed. + [Stefan Eissing] + *) mod_ssl: for all ssl_engine_vars.c lookups, fall back to master connection if conn_rec itself holds no valid SSLConnRec*. Fixes PR58666. [Stefan Eissing] diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index af7b0ba2b3..ac8b4e046f 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -2970 +2971 diff --git a/modules/http2/h2_conn.c b/modules/http2/h2_conn.c index cf44fd7c1f..b9b6e87bf0 100644 --- a/modules/http2/h2_conn.c +++ b/modules/http2/h2_conn.c @@ -195,55 +195,30 @@ apr_status_t h2_conn_process(conn_rec *c, request_rec *r, server_rec *s) static void fix_event_conn(conn_rec *c, conn_rec *master); -static int SLAVE_CONN_25DEV_STYLE = 1; - conn_rec *h2_conn_create(conn_rec *master, apr_pool_t *pool) { - apr_socket_t *socket; conn_rec *c; AP_DEBUG_ASSERT(master); - if (SLAVE_CONN_25DEV_STYLE) { - /* This is like the slave connection creation from 2.5-DEV. A - * very efficient way - not sure how compatible this is, since - * the core hooks are no longer run. - * But maybe it's is better this way, not sure yet. - */ - c = (conn_rec *) apr_palloc(pool, sizeof(conn_rec)); - - memcpy(c, master, sizeof(conn_rec)); - c->id = (master->id & (long)pool); - c->master = master; - c->input_filters = NULL; - c->output_filters = NULL; - c->pool = pool; - } - else { - /* CAVEAT: it seems necessary to setup the conn_rec in the master - * connection thread. Other attempts crashed. - * HOWEVER: we setup the connection using the pools and other items - * from the master connection, since we do not want to allocate - * lots of resources here. - * Lets allocated pools and everything else when we actually start - * working on this new connection. - */ - /* Not sure about the scoreboard handle. Reusing the one from the main - * connection could make sense, is not really correct, but we cannot - * easily create new handles for our worker threads either. - * TODO - */ - socket = ap_get_module_config(master->conn_config, &core_module); - c = ap_run_create_connection(pool, master->base_server, - socket, - master->id^((long)pool), - master->sbh, - master->bucket_alloc); - } + /* This is like the slave connection creation from 2.5-DEV. A + * very efficient way - not sure how compatible this is, since + * the core hooks are no longer run. + * But maybe it's is better this way, not sure yet. + */ + c = (conn_rec *) apr_palloc(pool, sizeof(conn_rec)); if (c == NULL) { ap_log_perror(APLOG_MARK, APLOG_ERR, APR_ENOMEM, pool, APLOGNO(02913) "h2_task: creating conn"); + return NULL; } + + memcpy(c, master, sizeof(conn_rec)); + c->id = (master->id & (long)pool); + c->master = master; + c->input_filters = NULL; + c->output_filters = NULL; + c->pool = pool; return c; } diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 18c481e782..e5a158c86a 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -900,7 +900,7 @@ apr_status_t h2_session_start(h2_session *session, int *rv) apr_status_t status = APR_SUCCESS; nghttp2_settings_entry settings[3]; size_t slen; - int i; + int win_size; AP_DEBUG_ASSERT(session); /* Start the conversation by submitting our SETTINGS frame */ @@ -963,10 +963,10 @@ apr_status_t h2_session_start(h2_session *session, int *rv) settings[slen].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; settings[slen].value = (uint32_t)session->max_stream_count; ++slen; - i = h2_config_geti(session->config, H2_CONF_WIN_SIZE); - if (i != H2_INITIAL_WINDOW_SIZE) { + win_size = h2_config_geti(session->config, H2_CONF_WIN_SIZE); + if (win_size != H2_INITIAL_WINDOW_SIZE) { settings[slen].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; - settings[slen].value = i; + settings[slen].value = win_size; ++slen; } @@ -978,7 +978,25 @@ apr_status_t h2_session_start(h2_session *session, int *rv) APLOGNO(02935) "nghttp2_submit_settings: %s", nghttp2_strerror(*rv)); } - + else { + /* use maximum possible value for connection window size. We are only + * interested in per stream flow control. which have the initial window + * size configured above. + * Therefore, for our use, the connection window can only get in the + * way. Example: if we allow 100 streams with a 32KB window each, we + * buffer up to 3.2 MB of data. Unless we do separate connection window + * interim updates, any smaller connection window will lead to blocking + * in DATA flow. + */ + *rv = nghttp2_submit_window_update(session->ngh2, NGHTTP2_FLAG_NONE, + 0, NGHTTP2_MAX_WINDOW_SIZE - win_size); + if (*rv != 0) { + status = APR_EGENERAL; + ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c, + APLOGNO(02970) "nghttp2_submit_window_update: %s", + nghttp2_strerror(*rv)); + } + } return status; } @@ -1288,7 +1306,7 @@ struct h2_stream *h2_session_push(h2_session *session, h2_stream *is, session->id, is->id, nghttp2_strerror(nid)); return NULL; } - + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, "h2_stream(%ld-%d): promised new stream %d for %s %s on %d", session->id, is->id, nid, @@ -1305,6 +1323,7 @@ struct h2_stream *h2_session_push(h2_session *session, h2_stream *is, h2_stream_cleanup(stream); stream = NULL; } + ++session->unsent_promises; } else { ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, @@ -1555,14 +1574,23 @@ apr_status_t h2_session_process(h2_session *session) h2_session_resume_streams_with_data(session); if (h2_stream_set_has_unsubmitted(session->streams)) { + int unsent_submits = 0; + /* If we have responses ready, submit them now. */ while ((stream = h2_mplx_next_submit(session->mplx, session->streams))) { status = submit_response(session, stream); + ++unsent_submits; + + /* Unsent push promises are written immediately, as nghttp2 + * 1.5.0 realizes internal stream data structures only on + * send and we might need them for other submits. + * Also, to conserve memory, we send at least every 10 submits + * so that nghttp2 does not buffer all outbound items too + * long. + */ if (status == APR_SUCCESS - && nghttp2_session_want_write(session->ngh2)) { - int rv; - - rv = nghttp2_session_send(session->ngh2); + && (session->unsent_promises || unsent_submits > 10)) { + int rv = nghttp2_session_send(session->ngh2); if (rv != 0) { ap_log_cerror( APLOG_MARK, APLOG_DEBUG, 0, session->c, "h2_session: send: %s", nghttp2_strerror(rv)); @@ -1574,6 +1602,8 @@ apr_status_t h2_session_process(h2_session *session) else { have_written = 1; wait_micros = 0; + session->unsent_promises = 0; + unsent_submits = 0; } } } @@ -1598,6 +1628,7 @@ apr_status_t h2_session_process(h2_session *session) else { have_written = 1; wait_micros = 0; + session->unsent_promises = 0; } } diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h index 00347c93e4..cc2608089a 100644 --- a/modules/http2/h2_session.h +++ b/modules/http2/h2_session.h @@ -63,6 +63,8 @@ struct h2_session { int aborted; /* this session is being aborted */ int reprioritize; /* scheduled streams priority needs to * be re-evaluated */ + int unsent_promises; /* number of submitted, but not yet sent + * push promised */ apr_size_t frames_received; /* number of http/2 frames received */ apr_size_t max_stream_count; /* max number of open streams */ apr_size_t max_stream_mem; /* max buffer memory for a single stream */