[PATCH 08 of 10] QUIC: idle mode for main connection
Roman Arutyunyan
arut at nginx.com
Thu Oct 20 14:25:03 UTC 2022
Hi,
On Thu, Oct 20, 2022 at 03:50:15PM +0400, Sergey Kandaurov wrote:
> On Thu, Sep 08, 2022 at 01:06:35PM +0400, Roman Arutyunyan wrote:
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at nginx.com>
> > # Date 1662627133 -14400
> > # Thu Sep 08 12:52:13 2022 +0400
> > # Branch quic
> > # Node ID e0634a484d9a2d82d43f565d64a0a22e989ac1cb
> > # Parent 1dd6fabfdcb5b52af495f9d8fc00f64ae36a537c
> > QUIC: idle mode for main connection.
> >
> > Now main QUIC connection for HTTP/3 always has c->idle flag set. This allows
> > the connection to receive worker shutdown notification. It is passed to
> > application level via a new conf->shutdown() callback.
> >
> > The HTTP/3 shutdown callback sends GOAWAY to client and gracefully shuts down
> > the QUIC connection.
> >
> > diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> > --- a/src/event/quic/ngx_event_quic.c
> > +++ b/src/event/quic/ngx_event_quic.c
> > @@ -341,6 +341,7 @@ ngx_quic_new_connection(ngx_connection_t
> > return NULL;
> > }
> >
> > + c->idle = 1;
> > ngx_reusable_connection(c, 1);
> >
> > ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
> > @@ -420,9 +421,9 @@ ngx_quic_input_handler(ngx_event_t *rev)
> > }
> >
> > if (c->close) {
> > - qc->error = NGX_QUIC_ERR_NO_ERROR;
> > - qc->error_reason = "graceful shutdown";
> > - ngx_quic_close_connection(c, NGX_ERROR);
> > + if (qc->conf->shutdown) {
>
> As previously discussed in private, this will need an additional check
> that we are not yet in qc->closing.
>
> > + qc->conf->shutdown(c);
> > + }
> > return;
> > }
Yes, added the check. Also, c->close is reset here similarly to HTTP/2
since we want to be able to handle future packets normally.
Also, current code which closes the connection instantly should remain for
connection reuse. To tell reuse from shutdown we can check ngx_exiting.
Assuming reuse does not make sense in shutdown mode, this will work good.
> > diff --git a/src/event/quic/ngx_event_quic.h b/src/event/quic/ngx_event_quic.h
> > --- a/src/event/quic/ngx_event_quic.h
> > +++ b/src/event/quic/ngx_event_quic.h
> > @@ -28,6 +28,9 @@
> > #define NGX_QUIC_STREAM_UNIDIRECTIONAL 0x02
> >
> >
> > +typedef void (*ngx_quic_shutdown_pt)(ngx_connection_t *c);
> > +
> > +
> > typedef enum {
> > NGX_QUIC_STREAM_SEND_READY = 0,
> > NGX_QUIC_STREAM_SEND_SEND,
> > @@ -74,6 +77,8 @@ typedef struct {
> > ngx_int_t stream_reject_code_uni;
> > ngx_int_t stream_reject_code_bidi;
> >
> > + ngx_quic_shutdown_pt shutdown;
> > +
> > u_char av_token_key[NGX_QUIC_AV_KEY_LEN];
> > u_char sr_token_key[NGX_QUIC_SR_KEY_LEN];
> > } ngx_quic_conf_t;
> > diff --git a/src/http/v3/ngx_http_v3.h b/src/http/v3/ngx_http_v3.h
> > --- a/src/http/v3/ngx_http_v3.h
> > +++ b/src/http/v3/ngx_http_v3.h
> > @@ -141,6 +141,7 @@ struct ngx_http_v3_session_s {
> > uint64_t next_push_id;
> > uint64_t max_push_id;
> > uint64_t goaway_push_id;
> > + uint64_t next_request_id;
> >
> > off_t total_bytes;
> > off_t payload_bytes;
> > @@ -158,6 +159,7 @@ void ngx_http_v3_init(ngx_connection_t *
> > void ngx_http_v3_reset_connection(ngx_connection_t *c);
> > ngx_int_t ngx_http_v3_init_session(ngx_connection_t *c);
> > ngx_int_t ngx_http_v3_check_flood(ngx_connection_t *c);
> > +void ngx_http_v3_shutdown(ngx_connection_t *c);
> >
> > ngx_int_t ngx_http_v3_read_request_body(ngx_http_request_t *r);
> > ngx_int_t ngx_http_v3_read_unbuffered_request_body(ngx_http_request_t *r);
> > diff --git a/src/http/v3/ngx_http_v3_module.c b/src/http/v3/ngx_http_v3_module.c
> > --- a/src/http/v3/ngx_http_v3_module.c
> > +++ b/src/http/v3/ngx_http_v3_module.c
> > @@ -249,6 +249,8 @@ ngx_http_v3_create_srv_conf(ngx_conf_t *
> > h3scf->quic.stream_reject_code_bidi = NGX_HTTP_V3_ERR_REQUEST_REJECTED;
> > h3scf->quic.active_connection_id_limit = NGX_CONF_UNSET_UINT;
> >
> > + h3scf->quic.shutdown = ngx_http_v3_shutdown;
> > +
> > return h3scf;
> > }
> >
> > diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> > --- a/src/http/v3/ngx_http_v3_request.c
> > +++ b/src/http/v3/ngx_http_v3_request.c
> > @@ -97,6 +97,37 @@ ngx_http_v3_init(ngx_connection_t *c)
> > }
> >
> >
> > +void
> > +ngx_http_v3_shutdown(ngx_connection_t *c)
> > +{
> > + ngx_http_v3_session_t *h3c;
>
> extra indent
>
> > +
> > + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 shutdown");
> > +
> > + h3c = ngx_http_v3_get_session(c);
> > +
> > + if (h3c == NULL) {
> > + ngx_quic_finalize_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> > + "connection shutdown");
> > + return;
> > + }
> > +
> > + if (!h3c->goaway) {
> > + h3c->goaway = 1;
> > +
> > +#if (NGX_HTTP_V3_HQ)
> > + if (!h3c->hq)
> > +#endif
> > + {
> > + (void) ngx_http_v3_send_goaway(c, h3c->next_request_id);
> > + }
> > +
> > + ngx_http_v3_shutdown_connection(c, NGX_HTTP_V3_ERR_NO_ERROR,
> > + "connection shutdown");
> > + }
>
> Note that this callback is used to be called from a read event as part of
> graceful shutdown.
> With ngx_quic_finalize_connection() remade in patch #4 (reusable mode)
> to defer closing QUIC connection to a posted event, this call now results
> in a posted event, which no one can fulfill, hence no further action until
> quic idle timeout fires.
> It could be fixed by executing known posted events after shutdown callback
> or more globally - as part of graceful shutdown itself.
Yes, events posted from ngx_close_idle_connections() are not handled right away.
Instead, they are handled at the end of the next cycle, which normally
happens after a timeout. There seems to be no pretty way to fix this, unless
we handle posted events in ngx_worker_process_cycle() right after
ngx_close_idle_connections(). We are trying to avoid global changes like this.
I suggest posting current connection read event as a next posted event. This
will effectively set next cycle timeout to be zero and eliminate the problem.
> > +}
> > +
> > +
> > static void
> > ngx_http_v3_init_request_stream(ngx_connection_t *c)
> > {
> > @@ -137,6 +168,8 @@ ngx_http_v3_init_request_stream(ngx_conn
> >
> > pc = c->quic->parent;
> >
> > + h3c->next_request_id = c->quic->id + 0x04;
> > +
> > if (n + 1 == clcf->keepalive_requests
> > || ngx_current_msec - pc->start_time > clcf->keepalive_time)
> > {
> > @@ -146,7 +179,7 @@ ngx_http_v3_init_request_stream(ngx_conn
> > if (!h3c->hq)
> > #endif
> > {
> > - if (ngx_http_v3_send_goaway(c, (n + 1) << 2) != NGX_OK) {
> > + if (ngx_http_v3_send_goaway(c, h3c->next_request_id) != NGX_OK) {
> > ngx_http_close_connection(c);
> > return;
> > }
> >
> > _______________________________________________
> > nginx-devel mailing list -- nginx-devel at nginx.org
> > To unsubscribe send an email to nginx-devel-leave at nginx.org
> _______________________________________________
> nginx-devel mailing list -- nginx-devel at nginx.org
> To unsubscribe send an email to nginx-devel-leave at nginx.org
Attached is a diff to the current patch.
--
Roman
-------------- next part --------------
# HG changeset patch
# User Roman Arutyunyan <arut at nginx.com>
# Date 1666273166 -14400
# Thu Oct 20 17:39:26 2022 +0400
# Branch quic
# Node ID d6c725081a0b024886822e1cc722fdace9c32621
# Parent a4ba2ac5fa55ef94bb75a66e66e0b19d792fed10
[mq]: quic-idle-fix1
diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
--- a/src/event/quic/ngx_event_quic.c
+++ b/src/event/quic/ngx_event_quic.c
@@ -421,9 +421,22 @@ ngx_quic_input_handler(ngx_event_t *rev)
}
if (c->close) {
- if (qc->conf->shutdown) {
+ c->close = 0;
+
+ if (!ngx_exiting) {
+ qc->error = NGX_QUIC_ERR_NO_ERROR;
+ qc->error_reason = "graceful shutdown";
+ ngx_quic_close_connection(c, NGX_ERROR);
+ return;
+ }
+
+ if (!qc->closing && qc->conf->shutdown) {
+ /* do not delay events posted by shutdown() */
+ ngx_post_event(rev, &ngx_posted_next_events);
+
qc->conf->shutdown(c);
}
+
return;
}
diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
--- a/src/http/v3/ngx_http_v3_request.c
+++ b/src/http/v3/ngx_http_v3_request.c
@@ -100,7 +100,7 @@ ngx_http_v3_init(ngx_connection_t *c)
void
ngx_http_v3_shutdown(ngx_connection_t *c)
{
- ngx_http_v3_session_t *h3c;
+ ngx_http_v3_session_t *h3c;
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http3 shutdown");
More information about the nginx-devel
mailing list