From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id AD84F96D86 for ; Fri, 1 Mar 2024 16:03:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 93E4233CCE for ; Fri, 1 Mar 2024 16:03:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 1 Mar 2024 16:03:21 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4EA1A48313 for ; Fri, 1 Mar 2024 16:03:21 +0100 (CET) From: Hannes Laimer To: pbs-devel@lists.proxmox.com Date: Fri, 1 Mar 2024 16:03:15 +0100 Message-Id: <20240301150315.12253-1-h.laimer@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.007 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-backup-proxy.rs, task.pid, maintenance.rs, datastore.rs] Subject: [pbs-devel] [PATCH proxmox-backup v2] datastore: remove datastore from internal cache based on maintenance mode 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: , X-List-Received-Date: Fri, 01 Mar 2024 15:03:22 -0000 We keep a DataStore cache, so ChunkStore's and lock files are kept by the proxy process and don't have to be reopened every time. However, for specific maintenance modes, e.g. 'offline', our process should not keep file in that datastore open. This clears the cache entry of a datastore if it is in a specific maintanance mode and the last task finished, which also drops any files still open by the process. Signed-off-by: Hannes Laimer Tested-by: Gabriel Goller Reviewed-by: Gabriel Goller --- v2, thanks @Gabriel: - improve comments - remove not needed &'s and .clone()'s pbs-api-types/src/maintenance.rs | 6 +++++ pbs-datastore/src/datastore.rs | 41 ++++++++++++++++++++++++++++-- pbs-datastore/src/task_tracking.rs | 23 ++++++++++------- src/api2/config/datastore.rs | 18 +++++++++++++ src/bin/proxmox-backup-proxy.rs | 8 ++++++ 5 files changed, 85 insertions(+), 11 deletions(-) diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs index 1b03ca94..a1564031 100644 --- a/pbs-api-types/src/maintenance.rs +++ b/pbs-api-types/src/maintenance.rs @@ -77,6 +77,12 @@ pub struct MaintenanceMode { } impl MaintenanceMode { + /// Used for deciding whether the datastore is cleared from the internal cache after the last + /// task finishes, so all open files are closed. + pub fn clear_from_cache(&self) -> bool { + self.ty == MaintenanceType::Offline + } + pub fn check(&self, operation: Option) -> Result<(), Error> { if self.ty == MaintenanceType::Delete { bail!("datastore is being deleted"); diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 2f0e5279..f26dff83 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -104,8 +104,27 @@ impl Clone for DataStore { impl Drop for DataStore { fn drop(&mut self) { if let Some(operation) = self.operation { - if let Err(e) = update_active_operations(self.name(), operation, -1) { - log::error!("could not update active operations - {}", e); + let mut last_task = false; + match update_active_operations(self.name(), operation, -1) { + Err(e) => log::error!("could not update active operations - {}", e), + Ok(updated_operations) => { + last_task = updated_operations.read + updated_operations.write == 0; + } + } + + // remove datastore from cache iff + // - last task finished, and + // - datastore is in a maintenance mode that mandates it + let remove_from_cache = last_task + && pbs_config::datastore::config() + .and_then(|(s, _)| s.lookup::("datastore", self.name())) + .map_or(false, |c| { + c.get_maintenance_mode() + .map_or(false, |m| m.clear_from_cache()) + }); + + if remove_from_cache { + DATASTORE_MAP.lock().unwrap().remove(self.name()); } } } @@ -193,6 +212,24 @@ impl DataStore { Ok(()) } + /// trigger clearing cache entries based on maintenance mode. Entries will only + /// be cleared iff there is no other task running, if there is, the end of the + /// last running task will trigger the clearing of the cache entry. + pub fn update_datastore_cache() -> Result<(), Error> { + let (config, _digest) = pbs_config::datastore::config()?; + for (store, (_, _)) in &config.sections { + let datastore: DataStoreConfig = config.lookup("datastore", store)?; + if datastore + .get_maintenance_mode() + .map_or(false, |m| m.clear_from_cache()) + { + let _ = DataStore::lookup_datastore(store, Some(Operation::Lookup)); + } + } + + Ok(()) + } + /// Open a raw database given a name and a path. /// /// # Safety diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs index 18fbd4ba..ec06a0bc 100644 --- a/pbs-datastore/src/task_tracking.rs +++ b/pbs-datastore/src/task_tracking.rs @@ -91,15 +91,23 @@ pub fn get_active_operations_locked( Ok((data, lock.unwrap())) } -pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> { +pub fn update_active_operations( + name: &str, + operation: Operation, + count: i64, +) -> Result { let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name)); let (_lock, options) = open_lock_file(name)?; let pid = std::process::id(); let starttime = procfs::PidStat::read_from_pid(Pid::from_raw(pid as pid_t))?.starttime; - let mut updated = false; + let mut updated_active_operations = match operation { + Operation::Read => ActiveOperationStats { read: 1, write: 0 }, + Operation::Write => ActiveOperationStats { read: 0, write: 1 }, + Operation::Lookup => ActiveOperationStats { read: 0, write: 0 }, + }; let mut updated_tasks: Vec = match file_read_optional_string(&path)? { Some(data) => serde_json::from_str::>(&data)? .iter_mut() @@ -108,12 +116,12 @@ pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> Some(stat) if pid == task.pid && stat.starttime != task.starttime => None, Some(_) => { if pid == task.pid { - updated = true; match operation { Operation::Read => task.active_operations.read += count, Operation::Write => task.active_operations.write += count, Operation::Lookup => (), // no IO must happen there }; + updated_active_operations = task.active_operations; } Some(task.clone()) } @@ -124,15 +132,11 @@ pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> None => Vec::new(), }; - if !updated { + if updated_tasks.is_empty() { updated_tasks.push(TaskOperations { pid, starttime, - active_operations: match operation { - Operation::Read => ActiveOperationStats { read: 1, write: 0 }, - Operation::Write => ActiveOperationStats { read: 0, write: 1 }, - Operation::Lookup => ActiveOperationStats { read: 0, write: 0 }, - }, + active_operations: updated_active_operations, }) } replace_file( @@ -141,4 +145,5 @@ pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> options, false, ) + .map(|_| updated_active_operations) } diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index d571334c..519fadc9 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -389,7 +389,9 @@ pub fn update_datastore( data.tuning = update.tuning; } + let mut maintenance_mode_changed = false; if update.maintenance_mode.is_some() { + maintenance_mode_changed = data.maintenance_mode != update.maintenance_mode; data.maintenance_mode = update.maintenance_mode; } @@ -403,6 +405,22 @@ pub fn update_datastore( jobstate::update_job_last_run_time("garbage_collection", &name)?; } + // tell the proxy it might have to clear a cache entry + if maintenance_mode_changed { + tokio::spawn(async { + if let Ok(proxy_pid) = + proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN) + { + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid); + let _ = proxmox_rest_server::send_raw_command( + sock, + "{\"command\":\"update-datastore-cache\"}\n", + ) + .await; + } + }); + } + Ok(()) } diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs index 5d5bcdd3..d5e3e810 100644 --- a/src/bin/proxmox-backup-proxy.rs +++ b/src/bin/proxmox-backup-proxy.rs @@ -289,6 +289,14 @@ async fn run() -> Result<(), Error> { Ok(Value::Null) })?; + // clear cache entries for datastores that are in a specific maintenance mode + command_sock.register_command("update-datastore-cache".to_string(), |_value| { + if let Err(err) = DataStore::update_datastore_cache() { + log::error!("could not trigger update datastore cache: {err}"); + } + Ok(Value::Null) + })?; + let connections = proxmox_rest_server::connection::AcceptBuilder::new() .debug(debug) .rate_limiter_lookup(Arc::new(lookup_rate_limiter)) -- 2.39.2