On 5/29/20 11:41 AM, Stefan Eissing wrote:
>
>
>> Am 29.05.2020 um 11:23 schrieb Ruediger Pluem <[email protected]>:
>>
>>
>>
>> On 5/29/20 10:09 AM, Stefan Eissing wrote:
>>> Looks good. Now I learned about the "ping" parameter...
>>
>> Committed as r1878264 with a tweaked comment to make clear what I do.
Thanks for backporting both
>>
>> Getting me to the next possible enhancement. I already had a patch but when
>> putting it to the mail I got doubts that it could work
>> due to the fact, that in most use cases HTTP/2 is encrypted.
>> In AJP a set ping parameter on the worker will also cause an AJP ping to be
>> sent as the first thing even on fresh connections and
>> we only wait for the timeout set in the parameter for a reply. The idea
>> behind this is that the backend might be able to deal with
>> a TCP handshake, but not with processing a request, because maybe all
>> processing threads / processes on the backend application
>> are busy. This way this can be detected quickly and early and we can sent
>> our request to a different backend in case of a load
>> balancing scenario.
>> With HTTP/2 we will likely have a TLS handshake first which likely already
>> requires a responding backend application. So it would
>> not work to wait only for ping timeout time on a ping reply as we will
>> already wait for the timeout set on the socket to get an
>> answer to our TLS client Hello. So the idea would be to already lower the
>> socket timeout to ping timeout before the TLS handshake
>> starts and reset it back after we received the ping from the backend.
>> Opinions?
>
> HTTP/2 also has an initial SETTINGS frame handshake. We could use the ping
> timeout on the socket until the first NGHTTP2_SETTINGS frame from the backend
> arrives on a new connection.
And now I learned about the HTTP/2 handshake :-). I haven't though about the
SETTINGS frame. So the high level idea would be:
1. If a fresh session in h2_proxy_session_setup is created and the ping
parameter is set on the worker change the socket timeout
to the one in ping, but tuck away the current socket timeout.
2. In on_frame_recv in the NGHTTP2_SETTINGS restore the old timeout (if it was
changed) and continue.
So something like the below?
Index: modules/http2/h2_proxy_session.c
===================================================================
--- modules/http2/h2_proxy_session.c (revision 1878265)
+++ modules/http2/h2_proxy_session.c (working copy)
@@ -203,6 +203,21 @@
case NGHTTP2_PUSH_PROMISE:
break;
case NGHTTP2_SETTINGS:
+ /*
+ * Check if we have saved a socket timeout before sending the
+ * SETTINGS frame in h2_proxy_session_setup to perform a quick
+ * "ping" on the backend. If yes, restore the saved timeout to
+ * the socket.
+ */
+ if (session->save_timeout != -1) {
+ apr_socket_t *socket = NULL;
+
+ socket = ap_get_conn_socket(session->c);
+ if (socket) {
+ apr_socket_timeout_set(socket, session->save_timeout);
+ session->save_timeout = -1;
+ }
+ }
if (frame->settings.niv > 0) {
n = nghttp2_session_get_remote_settings(ngh2,
NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
if (n > 0) {
@@ -634,6 +649,8 @@
session->input = apr_brigade_create(session->pool,
session->c->bucket_alloc);
session->output = apr_brigade_create(session->pool,
session->c->bucket_alloc);
+ session->save_timeout = -1;
+
nghttp2_session_callbacks_new(&cbs);
nghttp2_session_callbacks_set_on_frame_recv_callback(cbs,
on_frame_recv);
nghttp2_session_callbacks_set_on_data_chunk_recv_callback(cbs,
stream_response_data);
@@ -654,6 +671,22 @@
nghttp2_option_del(option);
nghttp2_session_callbacks_del(cbs);
+ /*
+ * If a ping parameter is set on the worker set the socket timeout to
+ * it to "use" the possible TLS handshake and the initial SETTINGS
+ * frame as kind of ping. Tuck away the old timeout to restore it, once
+ * the SETTING frame arrived from the backend.
+ */
+ if (p_conn->worker->s->ping_timeout_set) {
+ apr_socket_t *socket = NULL;
+
+ socket = ap_get_conn_socket(session->c);
+ if (socket) {
+ apr_socket_timeout_get(socket, &session->save_timeout);
+ apr_socket_timeout_set(socket,
p_conn->worker->s->ping_timeout);
+ }
+ }
+
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03362)
"setup session for %s", p_conn->hostname);
}
Index: modules/http2/h2_proxy_session.h
===================================================================
--- modules/http2/h2_proxy_session.h (revision 1878265)
+++ modules/http2/h2_proxy_session.h (working copy)
@@ -94,6 +94,8 @@
apr_bucket_brigade *input;
apr_bucket_brigade *output;
+
+ apr_time_t save_timeout;
};
h2_proxy_session *h2_proxy_session_setup(const char *id, proxy_conn_rec
*p_conn,
Regards
Rüdiger
