Improve lock (#596)

* improve lock

Signed-off-by: Mu junxiang <1948535941@qq.com>

* feat(tests): add wait_for_object_absence helper and improve lifecycle test reliability

Signed-off-by: Mu junxiang <1948535941@qq.com>

* chore: remove dirty docs

Signed-off-by: Mu junxiang <1948535941@qq.com>

---------

Signed-off-by: Mu junxiang <1948535941@qq.com>
This commit is contained in:
guojidan
2025-09-28 08:57:56 +08:00
committed by GitHub
parent 23b40d398f
commit be66cf8bd3
9 changed files with 235 additions and 253 deletions
-1
View File
@@ -31,7 +31,6 @@ pub mod erasure_coding;
pub mod error;
pub mod file_cache;
pub mod global;
pub mod lock_utils;
pub mod metrics_realtime;
pub mod notification_sys;
pub mod pools;
-136
View File
@@ -1,136 +0,0 @@
// Copyright 2024 RustFS Team
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use crate::disk::endpoint::Endpoint;
use crate::error::Result;
use rustfs_lock::client::{LockClient, local::LocalClient, remote::RemoteClient};
use std::collections::HashMap;
use std::sync::Arc;
/// Create unique lock clients from endpoints
/// This function creates one client per unique host:port combination
/// to avoid duplicate connections to the same server
pub async fn create_unique_clients(endpoints: &[Endpoint]) -> Result<Vec<Arc<dyn LockClient>>> {
let mut unique_endpoints: HashMap<String, &Endpoint> = HashMap::new();
// Collect unique endpoints based on host:port
for endpoint in endpoints {
if endpoint.is_local {
// For local endpoints, use "local" as the key
unique_endpoints.insert("local".to_string(), endpoint);
} else {
// For remote endpoints, use host:port as the key
let host_port = format!(
"{}:{}",
endpoint.url.host_str().unwrap_or("localhost"),
endpoint.url.port().unwrap_or(9000)
);
unique_endpoints.insert(host_port, endpoint);
}
}
let mut clients = Vec::new();
// Create clients for unique endpoints
for (_key, endpoint) in unique_endpoints {
if endpoint.is_local {
// For local endpoints, create a local lock client
let local_client = LocalClient::new();
clients.push(Arc::new(local_client) as Arc<dyn LockClient>);
} else {
// For remote endpoints, create a remote lock client
let remote_client = RemoteClient::new(endpoint.url.to_string());
clients.push(Arc::new(remote_client) as Arc<dyn LockClient>);
}
}
Ok(clients)
}
#[cfg(test)]
mod tests {
use super::*;
use url::Url;
#[tokio::test]
async fn test_create_unique_clients_local() {
let endpoints = vec![
Endpoint {
url: Url::parse("http://localhost:9000").unwrap(),
is_local: true,
pool_idx: 0,
set_idx: 0,
disk_idx: 0,
},
Endpoint {
url: Url::parse("http://localhost:9000").unwrap(),
is_local: true,
pool_idx: 0,
set_idx: 0,
disk_idx: 1,
},
];
let clients = create_unique_clients(&endpoints).await.unwrap();
// Should only create one client for local endpoints
assert_eq!(clients.len(), 1);
assert!(clients[0].is_local().await);
}
#[tokio::test]
async fn test_create_unique_clients_mixed() {
let endpoints = vec![
Endpoint {
url: Url::parse("http://localhost:9000").unwrap(),
is_local: true,
pool_idx: 0,
set_idx: 0,
disk_idx: 0,
},
Endpoint {
url: Url::parse("http://remote1:9000").unwrap(),
is_local: false,
pool_idx: 0,
set_idx: 0,
disk_idx: 1,
},
Endpoint {
url: Url::parse("http://remote1:9000").unwrap(),
is_local: false,
pool_idx: 0,
set_idx: 0,
disk_idx: 2,
},
Endpoint {
url: Url::parse("http://remote2:9000").unwrap(),
is_local: false,
pool_idx: 0,
set_idx: 0,
disk_idx: 3,
},
];
let clients = create_unique_clients(&endpoints).await.unwrap();
// Should create 3 clients: 1 local + 2 unique remote
assert_eq!(clients.len(), 3);
// Check that we have one local client
let local_count = clients.iter().filter(|c| futures::executor::block_on(c.is_local())).count();
assert_eq!(local_count, 1);
// Check that we have two remote clients
let remote_count = clients.iter().filter(|c| !futures::executor::block_on(c.is_local())).count();
assert_eq!(remote_count, 2);
}
}
+25 -29
View File
@@ -4041,34 +4041,34 @@ impl StorageAPI for SetDisks {
del_errs.push(None)
}
// Use fast batch locking to acquire all locks atomically
let mut _guards: HashMap<String, rustfs_lock::FastLockGuard> = HashMap::new();
// Acquire locks in batch mode (best effort, matching previous behavior)
let mut batch = rustfs_lock::BatchLockRequest::new(self.locker_owner.as_str()).with_all_or_nothing(false);
let mut unique_objects: std::collections::HashSet<String> = std::collections::HashSet::new();
// Collect unique object names
for dobj in &objects {
unique_objects.insert(dobj.object_name.clone());
if unique_objects.insert(dobj.object_name.clone()) {
batch = batch.add_write_lock(bucket, dobj.object_name.clone());
}
}
// Acquire all locks in batch to prevent deadlocks
for object_name in unique_objects {
match self
.fast_lock_manager
.acquire_write_lock(bucket, object_name.as_str(), self.locker_owner.as_str())
.await
{
Ok(guard) => {
_guards.insert(object_name, guard);
}
Err(err) => {
let message = self.format_lock_error(bucket, object_name.as_str(), "write", &err);
// Mark all operations on this object as failed
for (i, dobj) in objects.iter().enumerate() {
if dobj.object_name == object_name {
del_errs[i] = Some(Error::other(message.clone()));
}
}
}
let batch_result = self.fast_lock_manager.acquire_locks_batch(batch).await;
let locked_objects: HashSet<String> = batch_result
.successful_locks
.iter()
.map(|key| key.object.as_ref().to_string())
.collect();
let _lock_guards = batch_result.guards;
let failed_map: HashMap<(String, String), rustfs_lock::fast_lock::LockResult> = batch_result
.failed_locks
.into_iter()
.map(|(key, err)| ((key.bucket.as_ref().to_string(), key.object.as_ref().to_string()), err))
.collect();
// Mark failures for objects that could not be locked
for (i, dobj) in objects.iter().enumerate() {
if let Some(err) = failed_map.get(&(bucket.to_string(), dobj.object_name.clone())) {
let message = self.format_lock_error(bucket, dobj.object_name.as_str(), "write", err);
del_errs[i] = Some(Error::other(message));
}
}
@@ -4137,7 +4137,7 @@ impl StorageAPI for SetDisks {
}
// Only add to vers_map if we hold the lock
if _guards.contains_key(&dobj.object_name) {
if locked_objects.contains(&dobj.object_name) {
vers_map.insert(&dobj.object_name, v);
}
}
@@ -4558,7 +4558,6 @@ impl StorageAPI for SetDisks {
};
// Acquire write-lock early; hold for the whole transition operation scope
// let mut _lock_guard: Option<rustfs_lock::LockGuard> = None;
// if !opts.no_lock {
// let guard_opt = self
// .namespace_lock
@@ -4687,7 +4686,6 @@ impl StorageAPI for SetDisks {
#[tracing::instrument(level = "debug", skip(self))]
async fn restore_transitioned_object(&self, bucket: &str, object: &str, opts: &ObjectOptions) -> Result<()> {
// Acquire write-lock early for the restore operation
// let mut _lock_guard: Option<rustfs_lock::LockGuard> = None;
// if !opts.no_lock {
// let guard_opt = self
// .namespace_lock
@@ -4772,7 +4770,6 @@ impl StorageAPI for SetDisks {
#[tracing::instrument(level = "debug", skip(self))]
async fn put_object_tags(&self, bucket: &str, object: &str, tags: &str, opts: &ObjectOptions) -> Result<ObjectInfo> {
// Acquire write-lock for tag update (metadata write)
// let mut _lock_guard: Option<rustfs_lock::LockGuard> = None;
// if !opts.no_lock {
// let guard_opt = self
// .namespace_lock
@@ -5433,7 +5430,6 @@ impl StorageAPI for SetDisks {
// let disks = Self::shuffle_disks(&disks, &fi.erasure.distribution);
// Acquire per-object exclusive lock via RAII guard. It auto-releases asynchronously on drop.
// let mut _object_lock_guard: Option<rustfs_lock::LockGuard> = None;
if let Some(http_preconditions) = opts.http_preconditions.clone() {
// if !opts.no_lock {
// let guard_opt = self
-4
View File
@@ -56,8 +56,6 @@ use tokio::time::Duration;
use tracing::warn;
use tracing::{error, info};
use crate::lock_utils::create_unique_clients;
#[derive(Debug, Clone)]
pub struct Sets {
pub id: Uuid,
@@ -164,8 +162,6 @@ impl Sets {
}
}
let _lock_clients = create_unique_clients(&set_endpoints).await?;
// Note: write_quorum was used for the old lock system, no longer needed with FastLock
let _write_quorum = set_drive_count - parity_count;
// Create fast lock manager for high performance