From: Stefan Reiter <s.reiter@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH v2 proxmox-backup 1/4] gc: avoid race between phase1 and forget/prune
Date: Thu, 15 Oct 2020 12:49:13 +0200 [thread overview]
Message-ID: <20201015104916.21170-2-s.reiter@proxmox.com> (raw)
In-Reply-To: <20201015104916.21170-1-s.reiter@proxmox.com>
...by saving all forgotten chunks into a HashSet when we detect a GC
phase1 currently running. If GC then hits a NotFound, it can check if
the snapshot was simply forgotten behind its back, or if an actual error
occurred and it needs to abort (since it might delete still referenced
chunks, if the error is transient and the index file is still there).
We have to attach the error message in {fixed,dynamic}_index via the
.context() method, otherwise the original std::io::Error gets lost and
we can't check for NotFound.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
v2:
* use File::open directly and catch std::io::Error that way, avoid Error.context
src/backup/datastore.rs | 62 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 7 deletions(-)
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 7dd2624c..ca8ca438 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -36,6 +36,9 @@ pub struct DataStore {
chunk_store: Arc<ChunkStore>,
gc_mutex: Mutex<bool>,
last_gc_status: Mutex<GarbageCollectionStatus>,
+
+ // bool indicates if phase1 is currently active
+ removed_during_gc: Mutex<(bool, HashSet<PathBuf>)>,
}
impl DataStore {
@@ -82,6 +85,7 @@ impl DataStore {
chunk_store: Arc::new(chunk_store),
gc_mutex: Mutex::new(false),
last_gc_status: Mutex::new(gc_status),
+ removed_during_gc: Mutex::new((false, HashSet::new())),
})
}
@@ -236,6 +240,14 @@ impl DataStore {
_guard = lock_dir_noblock(&full_path, "snapshot", "possibly running or in use")?;
}
+ // Acquire lock and keep it during remove operation, so there's no
+ // chance for a race between adding to the hash and actually removing
+ // the dir (phase1 might otherwise start in-between)
+ let mut removed_guard = self.removed_during_gc.lock().unwrap();
+ if removed_guard.0 {
+ removed_guard.1.insert(self.snapshot_path(&backup_dir));
+ }
+
log::info!("removing backup snapshot {:?}", full_path);
std::fs::remove_dir_all(&full_path)
.map_err(|err| {
@@ -461,12 +473,35 @@ impl DataStore {
tools::fail_on_shutdown()?;
if let Ok(archive_type) = archive_type(&path) {
- if archive_type == ArchiveType::FixedIndex {
- let index = self.open_fixed_reader(&path)?;
- self.index_mark_used_chunks(index, &path, status, worker)?;
- } else if archive_type == ArchiveType::DynamicIndex {
- let index = self.open_dynamic_reader(&path)?;
- self.index_mark_used_chunks(index, &path, status, worker)?;
+ let full_path = self.chunk_store.relative_path(&path);
+
+ match std::fs::File::open(&full_path) {
+ Ok(file) => {
+ if archive_type == ArchiveType::FixedIndex {
+ let index = FixedIndexReader::new(file)?;
+ self.index_mark_used_chunks(index, &path, status, worker)?;
+ } else if archive_type == ArchiveType::DynamicIndex {
+ let index = DynamicIndexReader::new(file)?;
+ self.index_mark_used_chunks(index, &path, status, worker)?;
+ }
+ },
+ Err(err) => {
+ if err.kind() == std::io::ErrorKind::NotFound {
+ let (_, removed_hash) = &*self.removed_during_gc.lock().unwrap();
+ let backup_dir = path.parent();
+ if backup_dir.is_some() && removed_hash.contains(backup_dir.unwrap()) {
+ // index file not found, but we know that it was deleted by
+ // a concurrent 'forget', so we can safely ignore
+ } else {
+ bail!(
+ "index file not found but hasn't been removed by forget/prune, aborting GC - {}",
+ err
+ )
+ }
+ } else {
+ bail!(err)
+ }
+ }
}
}
done += 1;
@@ -512,7 +547,20 @@ impl DataStore {
crate::task_log!(worker, "Start GC phase1 (mark used chunks)");
- self.mark_used_chunks(&mut gc_status, worker)?;
+ {
+ let mut guard = self.removed_during_gc.lock().unwrap();
+ guard.0 = true;
+ }
+ let mark_res = self.mark_used_chunks(&mut gc_status, worker);
+ {
+ let mut guard = self.removed_during_gc.lock().unwrap();
+ guard.0 = false;
+ guard.1.clear();
+ }
+
+ if let Err(err) = mark_res {
+ bail!(err);
+ }
crate::task_log!(worker, "Start GC phase2 (sweep unused chunks)");
self.chunk_store.sweep_unused_chunks(
--
2.20.1
next prev parent reply other threads:[~2020-10-15 10:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-15 10:49 [pbs-devel] [PATCH v2 0/4] Locking and rustdoc improvements Stefan Reiter
2020-10-15 10:49 ` Stefan Reiter [this message]
2020-10-16 6:26 ` [pbs-devel] [PATCH v2 proxmox-backup 1/4] gc: avoid race between phase1 and forget/prune Dietmar Maurer
2020-10-15 10:49 ` [pbs-devel] [PATCH v2 proxmox-backup 2/4] datastore: add manifest locking Stefan Reiter
2020-10-16 6:33 ` Dietmar Maurer
2020-10-16 7:37 ` Dietmar Maurer
2020-10-16 7:39 ` [pbs-devel] applied: " Dietmar Maurer
2020-10-15 10:49 ` [pbs-devel] [PATCH v2 proxmox-backup 3/4] rustdoc: add crate level doc Stefan Reiter
2020-10-16 7:47 ` [pbs-devel] applied: " Dietmar Maurer
2020-10-15 10:49 ` [pbs-devel] [PATCH v2 proxmox-backup 4/4] rustdoc: overhaul backup rustdoc and add locking table Stefan Reiter
2020-10-16 7:47 ` [pbs-devel] applied: " 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=20201015104916.21170-2-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