public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium
@ 2024-06-24 19:57 Stoiko Ivanov
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0 Stoiko Ivanov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stoiko Ivanov @ 2024-06-24 19:57 UTC (permalink / raw)
  To: pve-devel

This patchset fixes a small glitch that we noticed in a pom-setup, creating
regular snapshots, without cleaning them up regularly.

Eventually medium sync becomes quite slow.
After removing many snapshots and running garbage collection both on the
mirror as well as on the medium the run-time for the sync still took quite
long. strace showed that the process still walked through the directories
for each snapshot on the medium - they were not cleaned up after all the
files inside were removed.

tested the patch locally (which is the reason for patch 1/2).

Stoiko Ivanov (2):
  bump proxmox-time to 2.0
  medium: remove snapshot link directories not present in source mirror

 Cargo.toml     |  2 +-
 debian/control |  2 +-
 src/medium.rs  | 18 +++++++++++++++++-
 src/types.rs   |  2 +-
 4 files changed, 20 insertions(+), 4 deletions(-)

-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0
  2024-06-24 19:57 [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
@ 2024-06-24 19:57 ` Stoiko Ivanov
  2024-06-26  6:40   ` [pve-devel] applied: " Fabian Grünbichler
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror Stoiko Ivanov
  2024-07-09 10:50 ` [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
  2 siblings, 1 reply; 6+ messages in thread
From: Stoiko Ivanov @ 2024-06-24 19:57 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 Cargo.toml     | 2 +-
 debian/control | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index bf7d754..edcdb87 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -35,4 +35,4 @@ proxmox-section-config = "2"
 proxmox-serde = "0.1"
 proxmox-subscription = { version = "0.4.2", features = [ "api-types" ] }
 proxmox-sys = { version = "0.5", features = [ "timer" ] }
-proxmox-time = { version = "1.1.5" }
+proxmox-time = "2"
diff --git a/debian/control b/debian/control
index b8c8ddc..86b6cfb 100644
--- a/debian/control
+++ b/debian/control
@@ -31,7 +31,7 @@ Build-Depends: bash-completion,
                librust-proxmox-subscription-0.4+default-dev (>= 0.4.2~),
                librust-proxmox-sys-0.5+default-dev,
                librust-proxmox-sys-0.5+timer-dev,
-               librust-proxmox-time-1+default-dev (>= 1.1.5~),
+               librust-proxmox-time-2+default-dev,
                librust-regex-1+default-dev (>= 1.5.5-~~),
                librust-sequoia-openpgp-1+default-dev (>= 1.7-~~),
                librust-serde-1+default-dev,
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror
  2024-06-24 19:57 [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0 Stoiko Ivanov
@ 2024-06-24 19:57 ` Stoiko Ivanov
  2024-06-26 10:24   ` Fabian Grünbichler
  2024-07-09 10:50 ` [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
  2 siblings, 1 reply; 6+ messages in thread
From: Stoiko Ivanov @ 2024-06-24 19:57 UTC (permalink / raw)
  To: pve-devel

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 <s.ivanov@proxmox.com>
---
 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<Snapshot> =
+            mirror::list_snapshots(&mirror)?.iter().cloned().collect();
+        let target_snapshots: HashSet<Snapshot> = 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] applied: [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0 Stoiko Ivanov
@ 2024-06-26  6:40   ` Fabian Grünbichler
  0 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2024-06-26  6:40 UTC (permalink / raw)
  To: Proxmox VE development discussion

applied this one already :)

On June 24, 2024 9:57 pm, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  Cargo.toml     | 2 +-
>  debian/control | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index bf7d754..edcdb87 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -35,4 +35,4 @@ proxmox-section-config = "2"
>  proxmox-serde = "0.1"
>  proxmox-subscription = { version = "0.4.2", features = [ "api-types" ] }
>  proxmox-sys = { version = "0.5", features = [ "timer" ] }
> -proxmox-time = { version = "1.1.5" }
> +proxmox-time = "2"
> diff --git a/debian/control b/debian/control
> index b8c8ddc..86b6cfb 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -31,7 +31,7 @@ Build-Depends: bash-completion,
>                 librust-proxmox-subscription-0.4+default-dev (>= 0.4.2~),
>                 librust-proxmox-sys-0.5+default-dev,
>                 librust-proxmox-sys-0.5+timer-dev,
> -               librust-proxmox-time-1+default-dev (>= 1.1.5~),
> +               librust-proxmox-time-2+default-dev,
>                 librust-regex-1+default-dev (>= 1.5.5-~~),
>                 librust-sequoia-openpgp-1+default-dev (>= 1.7-~~),
>                 librust-serde-1+default-dev,
> -- 
> 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror Stoiko Ivanov
@ 2024-06-26 10:24   ` Fabian Grünbichler
  0 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2024-06-26 10:24 UTC (permalink / raw)
  To: Proxmox VE development discussion

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 <s.ivanov@proxmox.com>
> ---
>  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<Snapshot> =
> +            mirror::list_snapshots(&mirror)?.iter().cloned().collect();
> +        let target_snapshots: HashSet<Snapshot> = 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium
  2024-06-24 19:57 [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0 Stoiko Ivanov
  2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror Stoiko Ivanov
@ 2024-07-09 10:50 ` Stoiko Ivanov
  2 siblings, 0 replies; 6+ messages in thread
From: Stoiko Ivanov @ 2024-07-09 10:50 UTC (permalink / raw)
  To: pve-devel

superseded-by:
https://lists.proxmox.com/pipermail/pve-devel/2024-July/064545.html

On Mon, 24 Jun 2024 21:57:10 +0200
Stoiko Ivanov <s.ivanov@proxmox.com> wrote:

> This patchset fixes a small glitch that we noticed in a pom-setup, creating
> regular snapshots, without cleaning them up regularly.
> 
> Eventually medium sync becomes quite slow.
> After removing many snapshots and running garbage collection both on the
> mirror as well as on the medium the run-time for the sync still took quite
> long. strace showed that the process still walked through the directories
> for each snapshot on the medium - they were not cleaned up after all the
> files inside were removed.
> 
> tested the patch locally (which is the reason for patch 1/2).
> 
> Stoiko Ivanov (2):
>   bump proxmox-time to 2.0
>   medium: remove snapshot link directories not present in source mirror
> 
>  Cargo.toml     |  2 +-
>  debian/control |  2 +-
>  src/medium.rs  | 18 +++++++++++++++++-
>  src/types.rs   |  2 +-
>  4 files changed, 20 insertions(+), 4 deletions(-)
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-07-09 10:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-24 19:57 [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov
2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 1/2] bump proxmox-time to 2.0 Stoiko Ivanov
2024-06-26  6:40   ` [pve-devel] applied: " Fabian Grünbichler
2024-06-24 19:57 ` [pve-devel] [PATCH proxmox-offline-mirror 2/2] medium: remove snapshot link directories not present in source mirror Stoiko Ivanov
2024-06-26 10:24   ` Fabian Grünbichler
2024-07-09 10:50 ` [pve-devel] [PATCH proxmox-offline-mirror 0/2] remove snapshot directories vanished on source also on medium Stoiko Ivanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal