From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>,
Stefan Reiter <s.reiter@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 3/4] gc: remove .bad files on garbage collect
Date: Fri, 4 Sep 2020 14:20:17 +0200 [thread overview]
Message-ID: <2708e6fa-c5b5-aa6c-e1a4-0a210a51f5b2@proxmox.com> (raw)
In-Reply-To: <20200903141705.6344-4-s.reiter@proxmox.com>
On 03.09.20 16:17, Stefan Reiter wrote:
> diff --git a/src/backup/chunk_store.rs b/src/backup/chunk_store.rs
> index e1da5a8a..5c2fb29d 100644
> --- a/src/backup/chunk_store.rs
> +++ b/src/backup/chunk_store.rs
> @@ -187,7 +187,7 @@ impl ChunkStore {
> pub fn get_chunk_iterator(
> &self,
> ) -> Result<
> - impl Iterator<Item = (Result<tools::fs::ReadDirEntry, Error>, usize)> + std::iter::FusedIterator,
> + impl Iterator<Item = (Result<tools::fs::ReadDirEntry, Error>, usize, bool)> + std::iter::FusedIterator,
> Error
> > {
> use nix::dir::Dir;
> @@ -218,20 +218,26 @@ impl ChunkStore {
> match inner.next() {
> Some(Ok(entry)) => {
> // skip files if they're not a hash
> - let bytes = entry.file_name().to_bytes();
> - if bytes.len() != 64 {
> - continue;
> + let hash = {
> + let bytes = entry.file_name().to_bytes();
> + bytes.len() == 64 && bytes.iter().all(u8::is_ascii_hexdigit)
why change the check, we do not care for deleting a non hex named file,
this is a private controlled directory after all. I'd avoid doing extra
checks, their costs, even if small, are amplified a lot.
> + };
> +
> + if hash {
> + return Some((Ok(entry), percentage, false));
> + } else if let Ok(name) = entry.file_name().to_str() {
> + if name.ends_with(".bad") {
> + return Some((Ok(entry), percentage, true));
> + }
we only want to remove bad chunks if a good exists again, else we may
loose information - e.g., if the detected corruption was caused by bad
memory (RAM) not the chunk itself.
> [...]
> @@ -321,7 +327,20 @@ impl ChunkStore {
> let lock = self.mutex.lock();
>
> if let Ok(stat) = fstatat(dirfd, filename, nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW) {
> - if stat.st_atime < min_atime {
> + if bad {
> + let res = unsafe { libc::unlinkat(dirfd, filename.as_ptr(), 0) };
don't we have something for this already? Else I'd move this out to a closure
or local function
> + if res != 0 {
> + let err = nix::Error::last();
> + worker.warn(format!(
> + "unlink .bad file {:?} failed on store '{}' - {}",
"unlinking corrupt chunk ...
> + filename,
> + self.name,
> + err,
> + ));
> + }
> + status.removed_bad += 1;
> + status.removed_bytes += stat.st_size as u64;
you count this up even if the unlinkat failed?
> + } else if stat.st_atime < min_atime {
> //let age = now - stat.st_atime;
> //println!("UNLINK {} {:?}", age/(3600*24), filename);
> let res = unsafe { libc::unlinkat(dirfd, filename.as_ptr(), 0) };
next prev parent reply other threads:[~2020-09-04 12:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-03 14:17 [pbs-devel] [PATCH 0/4] Improve corrupt chunk handling Stefan Reiter
2020-09-03 14:17 ` [pbs-devel] [PATCH proxmox-backup 1/4] verify: fix log units Stefan Reiter
2020-09-03 14:17 ` [pbs-devel] [PATCH proxmox-backup 2/4] verify: rename corrupted chunks with .bad extension Stefan Reiter
2020-09-03 14:17 ` [pbs-devel] [PATCH proxmox-backup 3/4] gc: remove .bad files on garbage collect Stefan Reiter
2020-09-04 12:20 ` Thomas Lamprecht [this message]
2020-09-03 14:17 ` [pbs-devel] [PATCH proxmox-backup 4/4] backup: check all referenced chunks actually exist Stefan Reiter
2020-09-03 15:40 ` [pbs-devel] [PATCH 0/4] Improve corrupt chunk handling Dietmar Maurer
2020-09-03 15:51 ` Dietmar Maurer
2020-09-07 9:31 ` Stefan Reiter
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=2708e6fa-c5b5-aa6c-e1a4-0a210a51f5b2@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=s.reiter@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