public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [RFC PATCH proxmox-backup] tools/async_io: do not error on Accept for StaticIncoming
Date: Thu,  8 Apr 2021 14:28:20 +0200	[thread overview]
Message-ID: <20210408122820.13492-1-d.csapak@proxmox.com> (raw)

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





             reply	other threads:[~2021-04-08 12:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-08 12:28 Dominik Csapak [this message]
2021-04-12 13:55 ` [pbs-devel] applied: " Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210408122820.13492-1-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal