From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D52CE1FF16B for ; Thu, 9 Jan 2025 15:07:12 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6FA0E1009B; Thu, 9 Jan 2025 15:06:55 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Thu, 9 Jan 2025 15:06:21 +0100 Message-Id: <20250109140623.329770-4-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250109140623.329770-1-c.ebner@proxmox.com> References: <20250109140623.329770-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 v3 proxmox-backup 3/5] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 suppresses the disconnection error 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 --- changes since version 2: - use api macro for handler declaration src/api2/reader/environment.rs | 20 +++++++++++++++++++- src/api2/reader/mod.rs | 27 +++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 5 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, pub backup_dir: BackupDir, allowed_chunks: Arc>>, + connection_state: Arc>, +} + +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..5744f59a0 100644 --- a/src/api2/reader/mod.rs +++ b/src/api2/reader/mod.rs @@ -14,7 +14,7 @@ use proxmox_router::{ http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router, RpcEnvironment, SubdirMap, }; -use proxmox_schema::{BooleanSchema, ObjectSchema}; +use proxmox_schema::{api, BooleanSchema, ObjectSchema}; use proxmox_sortable_macro::sortable; use proxmox_sys::fs::lock_dir_noblock_shared; @@ -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,17 @@ fn download_chunk( .boxed() } +#[api] +/// Signal the reader instance finished successfully +fn finish( + _info: &ApiMethod, + rpcenv: &mut dyn RpcEnvironment, +) -> Result { + 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