From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 2EDBA1FF2AD
for <inbox@lore.proxmox.com>; 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
<pbs-devel@lists.proxmox.com>, Gabriel Goller <g.goller@proxmox.com>
References: <20240703150249.461110-1-g.goller@proxmox.com>
Content-Language: en-US, de-DE
From: Christian Ebner <c.ebner@proxmox.com>
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
<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>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
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 <g.goller@proxmox.com>
> ---
> 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 <group> --output-format json-pretty
```
So please consider:
Tested-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel