From 4f5a1cbf9175138dbd45e65c44b8a6bb4c04e657 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Tue, 12 Jul 2022 16:29:38 +0900 Subject: [PATCH] reconsider timeout for h3 connections --- src/constants.rs | 5 ++--- src/globals.rs | 4 +++- src/main.rs | 6 +++++- src/msg_handler/handler.rs | 11 +++++++++-- src/proxy/proxy_h3.rs | 37 +++++++++++++------------------------ src/proxy/proxy_main.rs | 7 +++---- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 9fef23b..94e97ce 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -2,7 +2,8 @@ pub const LISTEN_ADDRESSES_V4: &[&str] = &["0.0.0.0"]; pub const LISTEN_ADDRESSES_V6: &[&str] = &["[::]"]; // pub const HTTP_LISTEN_PORT: u16 = 8080; // pub const HTTPS_LISTEN_PORT: u16 = 8443; -pub const TIMEOUT_SEC: u64 = 10; +pub const PROXY_TIMEOUT_SEC: u64 = 10; +pub const UPSTREAM_TIMEOUT_SEC: u64 = 8; pub const MAX_CLIENTS: usize = 512; pub const MAX_CONCURRENT_STREAMS: u32 = 16; // #[cfg(feature = "tls")] @@ -10,5 +11,3 @@ pub const CERTS_WATCH_DELAY_SECS: u32 = 10; #[cfg(feature = "h3")] pub const H3_ALT_SVC_MAX_AGE: u32 = 60; -#[cfg(feature = "h3")] -pub const H3_CONN_TIMEOUT_MILLIS: u64 = 2000; diff --git a/src/globals.rs b/src/globals.rs index a1746a6..846390b 100644 --- a/src/globals.rs +++ b/src/globals.rs @@ -11,7 +11,9 @@ pub struct Globals { pub http_port: Option, pub https_port: Option, - pub timeout: Duration, + pub proxy_timeout: Duration, + pub upstream_timeout: Duration, + pub max_clients: usize, pub clients_count: ClientsCount, pub max_concurrent_streams: u32, diff --git a/src/main.rs b/src/main.rs index e0e4b6e..1abb2fd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -60,7 +60,11 @@ fn main() { http_port: None, https_port: None, http3: false, - timeout: Duration::from_secs(TIMEOUT_SEC), + + // TODO: Reconsider each timeout values + proxy_timeout: Duration::from_secs(PROXY_TIMEOUT_SEC), + upstream_timeout: Duration::from_secs(UPSTREAM_TIMEOUT_SEC), + max_clients: MAX_CLIENTS, clients_count: Default::default(), max_concurrent_streams: MAX_CONCURRENT_STREAMS, diff --git a/src/msg_handler/handler.rs b/src/msg_handler/handler.rs index b438d98..eaab9b9 100644 --- a/src/msg_handler/handler.rs +++ b/src/msg_handler/handler.rs @@ -8,7 +8,10 @@ use hyper::{ Body, Client, Request, Response, StatusCode, Uri, Version, }; use std::{net::SocketAddr, sync::Arc}; -use tokio::io::copy_bidirectional; +use tokio::{ + io::copy_bidirectional, + time::{timeout, Duration}, +}; #[derive(Clone)] pub struct HttpMessageHandler @@ -92,7 +95,11 @@ where // Forward request to let mut res_backend = { - match tokio::time::timeout(self.globals.timeout, self.forwarder.request(req_forwarded)).await + match timeout( + self.globals.upstream_timeout + Duration::from_secs(1), + self.forwarder.request(req_forwarded), + ) + .await { Err(_) => { return http_error(StatusCode::GATEWAY_TIMEOUT); diff --git a/src/proxy/proxy_h3.rs b/src/proxy/proxy_h3.rs index b6f286b..af9284c 100644 --- a/src/proxy/proxy_h3.rs +++ b/src/proxy/proxy_h3.rs @@ -1,10 +1,10 @@ use super::Proxy; -use crate::{constants::*, error::*, log::*}; +use crate::{error::*, log::*}; use bytes::{Buf, Bytes}; use h3::{quic::BidiStream, server::RequestStream}; use hyper::{client::connect::Connect, Body, HeaderMap, Request, Response}; use std::net::SocketAddr; -use tokio::time::Duration; +use tokio::time::{timeout, Duration}; impl Proxy where @@ -17,10 +17,9 @@ where return; } let fut = self.clone().handle_connection_h3(conn); - let timeout_sec = self.globals.timeout; self.globals.runtime_handle.spawn(async move { - if let Err(e) = tokio::time::timeout(timeout_sec + Duration::from_secs(1), fut).await { - // TODO: ここのtimeoutはどの値を使うべき？ + // Timeout is based on underlying quic + if let Err(e) = fut.await { warn!("QUIC or HTTP/3 connection failed: {}", e) } clients_count.decrement(); @@ -54,22 +53,16 @@ where .await?; info!("HTTP/3 connection established"); - // TODO: Work around for timeout... + // Does this work enough? // while let Some((req, stream)) = h3_conn // .accept() // .await // .map_err(|e| anyhow!("HTTP/3 accept failed: {}", e))? - while let Some((req, stream)) = match tokio::time::timeout( - Duration::from_millis(H3_CONN_TIMEOUT_MILLIS), - h3_conn.accept(), - ) - .await - { - Ok(r) => r.map_err(|e| anyhow!("HTTP/3 accept failed: {}", e))?, + while let Some((req, stream)) = match h3_conn.accept().await { + Ok(opt_req) => opt_req, Err(_) => { - warn!("No incoming stream after connection establishment / previous use"); - h3_conn.shutdown(0).await?; - return Ok(()); + warn!("HTTP/3 failed to accept incoming connection (likely timeout)"); + return Ok(h3_conn.shutdown(0).await?); } } { debug!( @@ -81,24 +74,20 @@ where let self_inner = self.clone(); self.globals.runtime_handle.spawn(async move { - if let Err(e) = tokio::time::timeout( - self_inner.globals.timeout + Duration::from_secs(1), // timeout per stream + if let Err(e) = timeout( + self_inner.globals.proxy_timeout + Duration::from_secs(1), // timeout per stream are considered as same as one in http2 self_inner.handle_stream_h3(req, stream, client_addr), ) .await { - error!("HTTP/3 request failed: {}", e); + error!("HTTP/3 failed to process stream: {}", e); } - // // TODO: Work around for timeout - // if let Err(e) = h3_conn.shutdown(0).await { - // error!("HTTP/3 connection shutdown failed: {}", e); - // } - // debug!("HTTP/3 connection shutdown (currently shutdown each time as work around for timeout)"); }); } } Err(err) => { warn!("QUIC accepting connection failed: {:?}", err); + return Err(anyhow!("{}", err)); } } diff --git a/src/proxy/proxy_main.rs b/src/proxy/proxy_main.rs index 56378b5..366045c 100644 --- a/src/proxy/proxy_main.rs +++ b/src/proxy/proxy_main.rs @@ -6,7 +6,7 @@ use tokio::{ io::{AsyncRead, AsyncWrite}, net::TcpListener, runtime::Handle, - time::Duration, + time::{timeout, Duration}, }; #[derive(Clone, Debug)] @@ -57,8 +57,8 @@ where // let handler_inner = self.msg_handler.clone(); self.globals.runtime_handle.clone().spawn(async move { - tokio::time::timeout( - self.globals.timeout + Duration::from_secs(1), + timeout( + self.globals.proxy_timeout + Duration::from_secs(1), server .serve_connection( stream, @@ -106,7 +106,6 @@ where let server = server.with_executor(executor); if self.tls_enabled { - // #[cfg(feature = "tls")] self.start_with_tls(server).await?; } else { self.start_without_tls(server).await?;