From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id A4DF61FF161
for <inbox@lore.proxmox.com>; Wed, 4 Dec 2024 09:32:34 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 3E1CF1F66;
Wed, 4 Dec 2024 09:32:34 +0100 (CET)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 4 Dec 2024 09:31:48 +0100
Message-Id: <20241204083149.58754-3-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20241204083149.58754-1-c.ebner@proxmox.com>
References: <20241204083149.58754-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.030 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH v2 proxmox-backup 2/3] api: reader: handle
reader client disconnects
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
Currently, if a reader client disconnects after finishing its work,
the connection will be closed by the client without notifying the
server. The future handling the connection on then server side will
then return with a connection error, and in consequence the reader
worker task will log with error state. This can cause confusion [0],
as this is not an error but normal behaviour.
Instead of failing, provide an api endpoint for the client to signal
it has finished operation. The server sets the connection environment
state accordingly, and the connection error is suppressed if the flag
has been set. This follows the same logic used for the backup writer,
introduced by commit b428af97 ("backup: avoid Transport endpoint is
not connected error").
Report in the community forum:
[0] https://forum.proxmox.com/threads/158306/
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- Use the same approach as used for the backup writer, as the
connection graceful_shutdown did not resolve the issue at hand.
src/api2/reader/environment.rs | 20 +++++++++++++++++++-
src/api2/reader/mod.rs | 30 +++++++++++++++++++++++++++---
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/src/api2/reader/environment.rs b/src/api2/reader/environment.rs
index 3b2f06f43..3cdc8e394 100644
--- a/src/api2/reader/environment.rs
+++ b/src/api2/reader/environment.rs
@@ -1,5 +1,5 @@
use std::collections::HashSet;
-use std::sync::{Arc, RwLock};
+use std::sync::{Arc, Mutex, RwLock};
use serde_json::{json, Value};
@@ -24,6 +24,11 @@ pub struct ReaderEnvironment {
pub datastore: Arc<DataStore>,
pub backup_dir: BackupDir,
allowed_chunks: Arc<RwLock<HashSet<[u8; 32]>>>,
+ connection_state: Arc<Mutex<ConnectionState>>,
+}
+
+struct ConnectionState {
+ client_finished: bool,
}
impl ReaderEnvironment {
@@ -44,6 +49,9 @@ impl ReaderEnvironment {
formatter: JSON_FORMATTER,
backup_dir,
allowed_chunks: Arc::new(RwLock::new(HashSet::new())),
+ connection_state: Arc::new(Mutex::new(ConnectionState {
+ client_finished: false,
+ })),
}
}
@@ -69,6 +77,16 @@ impl ReaderEnvironment {
pub fn check_chunk_access(&self, digest: [u8; 32]) -> bool {
self.allowed_chunks.read().unwrap().contains(&digest)
}
+
+ pub(crate) fn client_finished(&self) -> bool {
+ let state = self.connection_state.lock().unwrap();
+ state.client_finished
+ }
+
+ pub(crate) fn finish(&self) {
+ let mut state = self.connection_state.lock().unwrap();
+ state.client_finished = true;
+ }
}
impl RpcEnvironment for ReaderEnvironment {
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index 50f80de43..cb53f6b5e 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -192,9 +192,16 @@ fn upgrade_to_backup_reader_protocol(
http.http2_initial_connection_window_size(window_size);
http.http2_max_frame_size(4 * 1024 * 1024);
- http.serve_connection(conn, service)
- .map_err(Error::from)
- .await
+ if let Err(err) = http.serve_connection(conn, service).await {
+ // Avoid Transport endpoint is not connected (os error 107)
+ // fixme: find a better way to test for that error
+ if !(err.to_string().starts_with("connection error")
+ && env2.client_finished())
+ {
+ return Err(Error::from(err));
+ }
+ }
+ Ok(())
};
futures::select! {
@@ -228,6 +235,7 @@ const READER_API_SUBDIRS: SubdirMap = &[
"download",
&Router::new().download(&API_METHOD_DOWNLOAD_FILE),
),
+ ("finish", &Router::new().post(&API_METHOD_FINISH)),
("speedtest", &Router::new().download(&API_METHOD_SPEEDTEST)),
];
@@ -347,6 +355,22 @@ fn download_chunk(
.boxed()
}
+#[sortable]
+pub const API_METHOD_FINISH: ApiMethod = ApiMethod::new(
+ &ApiHandler::Sync(&finish),
+ &ObjectSchema::new("Signal the reader instance is finished", &[]),
+);
+
+fn finish(
+ _param: Value,
+ _info: &ApiMethod,
+ rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Value, Error> {
+ let env: &ReaderEnvironment = rpcenv.as_ref();
+ env.finish();
+ Ok(Value::Null)
+}
+
/* this is too slow
fn download_chunk_old(
_parts: Parts,
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel