From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 53FC461041
for <pbs-devel@lists.proxmox.com>; Fri, 4 Sep 2020 14:20:50 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 41C3A208DF
for <pbs-devel@lists.proxmox.com>; Fri, 4 Sep 2020 14:20:20 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[212.186.127.180])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id 02994208D2
for <pbs-devel@lists.proxmox.com>; Fri, 4 Sep 2020 14:20:19 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C7FF844A1A
for <pbs-devel@lists.proxmox.com>; Fri, 4 Sep 2020 14:20:18 +0200 (CEST)
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>, Stefan Reiter <s.reiter@proxmox.com>
References: <20200903141705.6344-1-s.reiter@proxmox.com>
<20200903141705.6344-4-s.reiter@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Message-ID: <2708e6fa-c5b5-aa6c-e1a4-0a210a51f5b2@proxmox.com>
Date: Fri, 4 Sep 2020 14:20:17 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:81.0) Gecko/20100101
Thunderbird/81.0
MIME-Version: 1.0
In-Reply-To: <20200903141705.6344-4-s.reiter@proxmox.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
X-SPAM-LEVEL: Spam detection results: 0
AWL 1.072 Adjusted score from AWL reputation of From: address
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -2.403 Looks like a legit reply (A)
RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/,
medium trust
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] [PATCH proxmox-backup 3/4] gc: remove .bad files on
garbage collect
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Fri, 04 Sep 2020 12:20:50 -0000
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 =3D (Result<tools::fs::ReadDirEntry, Error>=
, usize)> + std::iter::FusedIterator,
> + impl Iterator<Item =3D (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)) =3D> {
> // skip files if they're not a hash
> - let bytes =3D entry.file_name().to_bytes()=
;
> - if bytes.len() !=3D 64 {
> - continue;
> + let hash =3D {
> + let bytes =3D entry.file_name().to_byt=
es();
> + bytes.len() =3D=3D 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, fa=
lse));
> + } else if let Ok(name) =3D 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 =3D self.mutex.lock();
> =20
> if let Ok(stat) =3D fstatat(dirfd, filename, nix::fcntl::A=
tFlags::AT_SYMLINK_NOFOLLOW) {
> - if stat.st_atime < min_atime {
> + if bad {
> + let res =3D unsafe { libc::unlinkat(dirfd, filenam=
e.as_ptr(), 0) };
don't we have something for this already? Else I'd move this out to a clo=
sure
or local function
> + if res !=3D 0 {
> + let err =3D nix::Error::last();
> + worker.warn(format!(
> + "unlink .bad file {:?} failed on store '{}=
' - {}",
"unlinking corrupt chunk ...
> + filename,
> + self.name,
> + err,
> + ));
> + }
> + status.removed_bad +=3D 1;
> + status.removed_bytes +=3D stat.st_size as u64;
you count this up even if the unlinkat failed?
> + } else if stat.st_atime < min_atime {
> //let age =3D now - stat.st_atime;
> //println!("UNLINK {} {:?}", age/(3600*24), filen=
ame);
> let res =3D unsafe { libc::unlinkat(dirfd, filenam=
e.as_ptr(), 0) };