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 50BF296479 for ; Thu, 29 Feb 2024 18:14:03 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 263E21739C for ; Thu, 29 Feb 2024 18:13:33 +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 ; Thu, 29 Feb 2024 18:13:32 +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 D529A480FE for ; Thu, 29 Feb 2024 18:13:31 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 29 Feb 2024 18:13:31 +0100 Message-Id: To: "Proxmox Backup Server development discussion" From: "Gabriel Goller" X-Mailer: aerc 0.17.0-37-g3aa8b6308482-dirty References: <20240222103319.49966-1-h.laimer@proxmox.com> In-Reply-To: <20240222103319.49966-1-h.laimer@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.097 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 - Subject: Re: [pbs-devel] [PATCH proxmox-backup] 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: Thu, 29 Feb 2024 17:14:03 -0000 Tested this thoroughly and it works as advertised! Just some small nits inline: > impl MaintenanceMode { > + /// Used for deciding weather the datastore is cleared from the inte= rnal cache after the last Small typo: weather -> whether > + // remove datastore from cache iff last task finished > + // and datastore is in a maintenance mode that says it shoul= d be Did something get cut off here? The sentence doesn't sound correct... > + let remove_from_cache =3D last_task > + && pbs_config::datastore::config() > + .and_then(|(s, _)| s.lookup::("data= store", &self.name())) `self.name()` is enough here, we don't need `&self.name()` > + pub fn update_datastore_cache() -> Result<(), Error> { > + let (config, _digest) =3D pbs_config::datastore::config()?; > + for (store, (_, _)) in &config.sections { > + let datastore: DataStoreConfig =3D config.lookup("datastore"= , &store)?; `store` is already a &String, we don't need to write `&store` > + if datastore > + .get_maintenance_mode() > + .map_or(false, |m| m.clear_from_cache()) > + { > + let _ =3D DataStore::lookup_datastore(&store, Some(Opera= tion::Lookup)); Same here > match operation { > Operation::Read =3D> task.active_operati= ons.read +=3D count, > Operation::Write =3D> task.active_operat= ions.write +=3D count, > Operation::Lookup =3D> (), // no IO must= happen there > }; > + updated_active_operations =3D task.active_op= erations.clone(); You can remove the `.clone()` call here Consider: Tested-by: Gabriel Goller Reviewed-by: Gabriel Goller