From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 86C191FF385 for ; Wed, 26 Jun 2024 12:24:49 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 54D6C1DB8D; Wed, 26 Jun 2024 12:24:59 +0200 (CEST) Date: Wed, 26 Jun 2024 12:24:51 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20240624195712.962468-1-s.ivanov@proxmox.com> <20240624195712.962468-3-s.ivanov@proxmox.com> In-Reply-To: <20240624195712.962468-3-s.ivanov@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1719384047.tajdf1clb3.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.052 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: [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" as discussed off-list, we might want to offer a "--remove-orphaned-files" mode for sync and gc, and clean up orphaned dirs (need to be detected first) and files (already are) in addition to this. this patch already gets rid of the orphaned dirs created during regular operations with a very low chance of false positives, but an additional opt-in check in remove_dir so that only (recursively) empty dir trees are removed would be great! On June 24, 2024 9:57 pm, Stoiko Ivanov wrote: > Currently a medium sync takes care to remove all files and the links > in the individual snapshot directories, but leaves the empty snapshot > hierarchy in place. This causes a medium sync to take longer than > necessary, as we walk through the complete mirror on the medium, > including all empty link directories. > > The status output is misleading as well, still showing more snapshots > than actually present (in the sense of containing any files). > > Noticed on a system, which creates snapshots on a daily basis, after a > cleanup removing many snapshots and running garbage collection on the > mirror and the medium, still caused the medium sync to be quite slow. > > Signed-off-by: Stoiko Ivanov > --- > src/medium.rs | 18 +++++++++++++++++- > src/types.rs | 2 +- > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/src/medium.rs b/src/medium.rs > index 53df50d..2c2d24e 100644 > --- a/src/medium.rs > +++ b/src/medium.rs > @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize}; > > use crate::{ > config::{self, ConfigLockGuard, MediaConfig, MirrorConfig}, > - generate_repo_file_line, > + generate_repo_file_line, mirror, > mirror::pool, > pool::Pool, > types::{Diff, Snapshot, SNAPSHOT_REGEX}, > @@ -404,6 +404,22 @@ pub fn sync( > Pool::create(&mirror_base, &mirror_pool)? > }; > > + let source_snapshots: HashSet = > + mirror::list_snapshots(&mirror)?.iter().cloned().collect(); > + let target_snapshots: HashSet = list_snapshots(medium_base, &mirror.id)? > + .iter() > + .cloned() > + .collect(); > + let target_only_snapshots = target_snapshots.difference(&source_snapshots); > + for snapshot in target_only_snapshots.into_iter() { > + let path = target_pool.get_path(Path::new(&snapshot.to_string()))?; > + println!( > + "Removing snapshot no longer present on mirror: {:?}", > + &snapshot.to_string() > + ); > + target_pool.lock()?.remove_dir(&path)?; > + } > + > let source_pool: Pool = pool(&mirror)?; > source_pool.lock()?.sync_pool(&target_pool, medium.verify)?; > > diff --git a/src/types.rs b/src/types.rs > index 7544d5e..94eee79 100644 > --- a/src/types.rs > +++ b/src/types.rs > @@ -71,7 +71,7 @@ const_regex! { > type: String, > format: &ApiStringFormat::Pattern(&SNAPSHOT_REGEX), > )] > -#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)] > +#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Hash)] > /// Mirror snapshot > pub struct Snapshot(i64); > > -- > 2.39.2 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel