From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 0D4E01FF15C
for <inbox@lore.proxmox.com>; Wed, 26 Mar 2025 12:44:28 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id CE92735B8B;
Wed, 26 Mar 2025 12:44:22 +0100 (CET)
From: Shannon Sterz <s.sterz@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 26 Mar 2025 12:44:13 +0100
Message-Id: <20250326114414.132478-4-s.sterz@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250326114414.132478-1-s.sterz@proxmox.com>
References: <20250326114414.132478-1-s.sterz@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.983 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
KAM_MAILER 2 Automated Mailer Tag Left in Email
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH proxmox-backup v9 3/4] fix #3935: datastore:
move manifest locking to new locking method
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>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
adds double stat'ing and removes directory hierarchy to bring manifest
locking in-line with other locks used by the BackupDir trait.
if the old locking mechanism is still supposed to be used, this still
falls back to the previous lock file. however, we already add double
stat'ing since it is trivial to do here and should only provide better
safety when it comes to removing locks.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
pbs-datastore/src/backup_info.rs | 45 ++++++++++++++++++++------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 4e381142..396b6bde 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -469,25 +469,41 @@ impl BackupDir {
/// Returns the filename to lock a manifest
///
/// Also creates the basedir. The lockfile is located in
- /// '/run/proxmox-backup/locks/{datastore}/[ns/{ns}/]+{type}/{id}/{timestamp}.index.json.lck'
- fn manifest_lock_path(&self) -> Result<PathBuf, Error> {
- let mut path = PathBuf::from(&format!("/run/proxmox-backup/locks/{}", self.store.name()));
- path.push(self.relative_path());
+ /// `${DATASTORE_LOCKS_DIR}/${datastore name}/${lock_file_path_helper(rpath)}.index.json.lck`
+ /// where rpath is the relative path of the snapshot.
+ fn manifest_lock_path(&self) -> PathBuf {
+ let path = Path::new(DATASTORE_LOCKS_DIR).join(self.store.name());
- std::fs::create_dir_all(&path)?;
- let ts = self.backup_time_string();
- path.push(format!("{ts}{MANIFEST_LOCK_NAME}"));
+ let rpath = Path::new(self.dir.group.ty.as_str())
+ .join(&self.dir.group.id)
+ .join(&self.backup_time_string)
+ .join(MANIFEST_LOCK_NAME);
- Ok(path)
+ path.join(lock_file_path_helper(&self.ns, rpath))
}
/// Locks the manifest of a snapshot, for example, to update or delete it.
pub(crate) fn lock_manifest(&self) -> Result<BackupLockGuard, Error> {
- let path = self.manifest_lock_path()?;
+ let path = if *OLD_LOCKING {
+ // old manifest lock path
+ let path = Path::new(DATASTORE_LOCKS_DIR)
+ .join(self.store.name())
+ .join(self.relative_path());
- // actions locking the manifest should be relatively short, only wait a few seconds
- open_backup_lockfile(&path, Some(std::time::Duration::from_secs(5)), true)
- .map_err(|err| format_err!("unable to acquire manifest lock {:?} - {}", &path, err))
+ std::fs::create_dir_all(&path)?;
+
+ path.join(format!("{}{MANIFEST_LOCK_NAME}", self.backup_time_string()))
+ } else {
+ self.manifest_lock_path()
+ };
+
+ lock_helper(self.store.name(), &path, |p| {
+ // update_manifest should never take a long time, so if
+ // someone else has the lock we can simply block a bit
+ // and should get it soon
+ open_backup_lockfile(p, Some(Duration::from_secs(5)), true)
+ .with_context(|| format_err!("unable to acquire manifest lock {p:?}"))
+ })
}
/// Returns a file name for locking a snapshot.
@@ -562,10 +578,7 @@ impl BackupDir {
})?;
// remove no longer needed lock files
- if let Ok(path) = self.manifest_lock_path() {
- let _ = std::fs::remove_file(path); // ignore errors
- }
-
+ let _ = std::fs::remove_file(self.manifest_lock_path()); // ignore errors
let _ = std::fs::remove_file(self.lock_path()); // ignore errors
Ok(())
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel