From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 2EDBA1FF2AD for ; Thu, 4 Jul 2024 12:43:06 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AC56830A29; Thu, 4 Jul 2024 12:43:21 +0200 (CEST) Message-ID: <29ade279-d013-4d86-a663-23591e0a1800@proxmox.com> Date: Thu, 4 Jul 2024 12:42:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox Backup Server development discussion , Gabriel Goller References: <20240703150249.461110-1-g.goller@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20240703150249.461110-1-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 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: Re: [pbs-devel] [PATCH proxmox-backup] datastore: avoid calculating protected attribute twice 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" nit: s/calculating/checking/ as this is not really a calculation in that sense. But this does not warrant a new version. On 7/3/24 17:02, Gabriel Goller wrote: > The protected status of the snapshot is retrieved twice. This is slow > because it stat's the .protected file multiple times. > > Signed-off-by: Gabriel Goller > --- > src/api2/admin/datastore.rs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index f28fb97fa975..9c5ef7185b74 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -505,7 +505,7 @@ fn list_snapshots_blocking( > group: group.into(), > time: info.backup_dir.backup_time(), > }; > - let protected = info.backup_dir.is_protected(); > + let protected = info.protected; > > match get_all_snapshot_files(&info) { > Ok((manifest, files)) => { This looks good to me, although there is now a possibly increased delay between the check and closure invocation, as the closure call can be significantly later, e.g. when there are a lot of snapshots in the group to list. This is however not problematic, as callers do not rely on this and the avoided lookup for the file path existence is beneficial. Did test this by setting a snapshot to protected and verifying the listing is still as expected via: ``` proxmox-backup-client snapshot list --output-format json-pretty ``` So please consider: Tested-by: Christian Ebner Reviewed-by: Christian Ebner _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel