[pbs-devel] applied: [RFC PATCH proxmox-backup] tools/async_io: do not error on Accept for StaticIncoming

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

applied

On Thu, Apr 08, 2021 at 02:28:20PM +0200, Dominik Csapak wrote:
> in proxmox-backup-proxy, we log and discard any errors on 'accept',
> so that we can continue to server requests
> 
> in proxmox-backup-api, we just have the StaticIncoming that accepts,
> which will forward any errors from the underlying TcpListener
> 
> this patch also logs and discards the errors, like in the proxy.
> Otherwise it could happen that if the api-daemon has more files open
> than the proxy, it will shut itself down because of a
> 'too many open files' error if there are many open connections
> 
> (the service should also restart on exit i think, but this is
> a separate issue)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> two remaining issues/questions:
> 
> 1. maybe we want an accept loop in proxmox-backup-api akin to the one
>    in the proxy? this way we have better locality of the error logging,
>    more control of e.g. how many connections we accept, etc.
> 
> 2. is it wise to log every failed connection attempt? i can very easily
>    generate massive logspam with a single line of 'ab'. shouldn't we
>    e.g. only log once a second how many connections were not accepted?
>    this is for the api as well as the proxy
> 
>  src/tools/async_io.rs | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/tools/async_io.rs b/src/tools/async_io.rs
> index 997c02fa..844afaa9 100644
> --- a/src/tools/async_io.rs
> +++ b/src/tools/async_io.rs
> @@ -6,6 +6,7 @@ use std::pin::Pin;
>  use std::task::{Context, Poll};
>  
>  use futures::stream::{Stream, TryStream};
> +use futures::ready;
>  use tokio::io::{AsyncRead, AsyncWrite, ReadBuf};
>  use tokio::net::TcpListener;
>  use hyper::client::connect::Connection;
> @@ -108,10 +109,15 @@ impl hyper::server::accept::Accept for StaticIncoming {
>          self: Pin<&mut Self>,
>          cx: &mut Context,
>      ) -> Poll<Option<Result<Self::Conn, Self::Error>>> {
> -        match self.get_mut().0.poll_accept(cx) {
> -            Poll::Pending => Poll::Pending,
> -            Poll::Ready(Ok((conn, _addr))) => Poll::Ready(Some(Ok(conn))),
> -            Poll::Ready(Err(err)) => Poll::Ready(Some(Err(err))),
> +        let this = self.get_mut();
> +        loop {
> +            match ready!(this.0.poll_accept(cx)) {
> +                Ok((conn, _addr)) => return Poll::Ready(Some(Ok(conn))),
> +                Err(err) => {
> +                    eprintln!("error accepting connection: {}", err);
> +                    continue;
> +                }
> +            }
>          }
>      }
>  }
> -- 
> 2.20.1