From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <g.goller@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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: <CZHQC54Z023V.3EGPI7YN8JAQG@proxmox.com>
To: "Proxmox Backup Server development discussion"
<pbs-devel@lists.proxmox.com>
From: "Gabriel Goller" <g.goller@proxmox.com>
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
<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>
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::<DataStoreConfig>("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 <g.goller@proxmox.com>
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>