From: Stefan Reiter <s.reiter@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v2 proxmox-backup 7/7] prune: also check backup snapshot locks
Date: Tue, 11 Aug 2020 10:50:42 +0200 [thread overview]
Message-ID: <20200811085042.30686-8-s.reiter@proxmox.com> (raw)
In-Reply-To: <20200811085042.30686-1-s.reiter@proxmox.com>
...to avoid pruning running backups or backups used as base for others.
Unfinished backups that have no lock are considered broken and will
always be pruned.
The ignore_locks parameter is used for testing, since flocks are not
available in test environment.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
Once again, feel free to remove my comments, I just can't wrap my head around
that logic without them ;)
src/api2/admin/datastore.rs | 2 +-
src/backup/prune.rs | 50 ++++++++++++++++++++++-----------
src/bin/proxmox-backup-proxy.rs | 2 +-
tests/prune.rs | 6 ++--
4 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index fe72ea32..a987416e 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -636,7 +636,7 @@ fn prune(
let list = group.list_backups(&datastore.base_path())?;
- let mut prune_info = compute_prune_info(list, &prune_options)?;
+ let mut prune_info = compute_prune_info(list, &prune_options, &datastore.base_path(), false)?;
prune_info.reverse(); // delete older snapshots first
diff --git a/src/backup/prune.rs b/src/backup/prune.rs
index f7a87c5a..f642e7b3 100644
--- a/src/backup/prune.rs
+++ b/src/backup/prune.rs
@@ -1,12 +1,13 @@
use anyhow::{Error};
use std::collections::{HashMap, HashSet};
-use std::path::PathBuf;
+use std::path::{PathBuf, Path};
use chrono::{DateTime, Timelike, Datelike, Local};
use super::{BackupDir, BackupInfo};
+use crate::tools::fs::lock_dir_noblock;
-enum PruneMark { Keep, KeepPartial, Remove }
+enum PruneMark { Keep, Ignored, Remove }
fn mark_selections<F: Fn(DateTime<Local>, &BackupInfo) -> String> (
mark: &mut HashMap<PathBuf, PruneMark>,
@@ -45,26 +46,39 @@ fn mark_selections<F: Fn(DateTime<Local>, &BackupInfo) -> String> (
}
}
-fn remove_incomplete_snapshots(
+fn mark_incomplete_and_in_use_snapshots(
mark: &mut HashMap<PathBuf, PruneMark>,
list: &Vec<BackupInfo>,
+ base_path: &Path,
+ ignore_locks: bool,
) {
- let mut keep_unfinished = true;
for info in list.iter() {
- // backup is considered unfinished if there is no manifest
- if info.is_finished() {
- // There is a new finished backup, so there is no need
- // to keep older unfinished backups.
- keep_unfinished = false;
- } else {
- let backup_id = info.backup_dir.relative_path();
- if keep_unfinished { // keep first unfinished
- mark.insert(backup_id, PruneMark::KeepPartial);
- } else {
+ let backup_id = info.backup_dir.relative_path();
+ let mut full_path = base_path.to_owned();
+ full_path.push(backup_id.clone());
+
+ if ignore_locks || lock_dir_noblock(&full_path, "snapshot", "").is_ok() {
+ if !info.is_finished() {
+ // incomplete backup, but we can lock it, so it's not running -
+ // always remove to clean up broken backups
mark.insert(backup_id, PruneMark::Remove);
}
- keep_unfinished = false;
+ // if locking succeeds and the backup is complete, let the regular
+ // marking logic figure it out
+ // note that we don't need to keep any locks either - any newly
+ // started backup can only ever use the latest finished backup as
+ // base, which is kept anyway (since we always keep the latest, and
+ // this function already filters out any unfinished ones)
+ } else {
+ // backups we can't lock we have to ignore - note that this means in
+ // case a backup is running in a group, the first three snapshots
+ // will always be kept
+ // we cannot special case that though, since we don't know if a lock
+ // is in place for backup or forget, where in the latter case we
+ // must not mark it as "Keep", as otherwise we might end up with
+ // no backup for a given selection period
+ mark.insert(backup_id, PruneMark::Ignored);
}
}
}
@@ -173,13 +187,15 @@ impl PruneOptions {
pub fn compute_prune_info(
mut list: Vec<BackupInfo>,
options: &PruneOptions,
+ base_path: &Path,
+ ignore_locks: bool,
) -> Result<Vec<(BackupInfo, bool)>, Error> {
let mut mark = HashMap::new();
BackupInfo::sort_list(&mut list, false);
- remove_incomplete_snapshots(&mut mark, &list);
+ mark_incomplete_and_in_use_snapshots(&mut mark, &list, base_path, ignore_locks);
if let Some(keep_last) = options.keep_last {
mark_selections(&mut mark, &list, keep_last as usize, |_local_time, info| {
@@ -227,7 +243,7 @@ pub fn compute_prune_info(
let backup_id = info.backup_dir.relative_path();
let keep = match mark.get(&backup_id) {
Some(PruneMark::Keep) => true,
- Some(PruneMark::KeepPartial) => true,
+ Some(PruneMark::Ignored) => true,
_ => false,
};
(info, keep)
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 3f7bf3ec..8cd2e105 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -442,7 +442,7 @@ async fn schedule_datastore_prune() {
let groups = BackupGroup::list_groups(&base_path)?;
for group in groups {
let list = group.list_backups(&base_path)?;
- let mut prune_info = compute_prune_info(list, &prune_options)?;
+ let mut prune_info = compute_prune_info(list, &prune_options, &base_path, false)?;
prune_info.reverse(); // delete older snapshots first
worker.log(format!("Starting prune on store \"{}\" group \"{}/{}\"",
diff --git a/tests/prune.rs b/tests/prune.rs
index d9758ea7..31f48970 100644
--- a/tests/prune.rs
+++ b/tests/prune.rs
@@ -1,5 +1,5 @@
use anyhow::{Error};
-use std::path::PathBuf;
+use std::path::{PathBuf, Path};
use proxmox_backup::backup::*;
@@ -9,7 +9,9 @@ fn get_prune_list(
options: &PruneOptions,
) -> Vec<PathBuf> {
- let mut prune_info = compute_prune_info(list, options).unwrap();
+ // ignore locks, would always fail in test environment
+ // base_path is only used for locking so leave empty
+ let mut prune_info = compute_prune_info(list, options, &Path::new(""), true).unwrap();
prune_info.reverse();
--
2.20.1
next prev parent reply other threads:[~2020-08-11 8:51 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-11 8:50 [pbs-devel] [PATCH v2 0/7] More flocking and race elimination Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 1/7] src/tools/fs.rs: new helper lock_dir_noblock Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 2/7] src/backup/backup_info.rs: remove BackupGroup lock() Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 3/7] datastore: prevent in-use deletion with locks instead of heuristic Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 4/7] backup: flock snapshot on backup start Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 5/7] Revert "backup: ensure base snapshots are still available after backup" Stefan Reiter
2020-08-11 8:50 ` [pbs-devel] [PATCH v2 proxmox-backup 6/7] backup: lock base snapshot and ensure existance on finish Stefan Reiter
2020-08-11 8:50 ` Stefan Reiter [this message]
2020-08-11 9:35 ` [pbs-devel] [PATCH v2 proxmox-backup 7/7] prune: also check backup snapshot locks Dietmar Maurer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200811085042.30686-8-s.reiter@proxmox.com \
--to=s.reiter@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox