refactor: NamespaceLock (nslock), AHM→Heal Crate, and Lock/Clippy Fixes (#1664)

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: weisd <2057561+weisd@users.noreply.github.com>
Co-authored-by: houseme <housemecn@gmail.com>

crates/lock/src/client/local.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use std::collections::HashMap;
+use std::hash::{Hash, Hasher};
 use std::sync::Arc;
 use tokio::sync::RwLock;
 
@@ -21,23 +22,69 @@ use crate::{
     LockResponse, LockStats, LockStatus, LockType, Result,
 };
 
-/// Local lock client using FastLock
-#[derive(Debug, Clone)]
+/// Default shard count for guard storage (must be power of 2)
+const DEFAULT_GUARD_SHARD_COUNT: usize = 64;
+
+/// Local lock client using FastLock with sharded guard storage for better concurrency
+#[derive(Debug)]
 pub struct LocalClient {
-    guard_storage: Arc<RwLock<HashMap<LockId, FastLockGuard>>>,
+    /// Sharded guard storage to reduce lock contention
+    guard_storage: Vec<Arc<RwLock<HashMap<LockId, FastLockGuard>>>>,
+    /// Mask for fast shard index calculation (shard_count - 1)
+    shard_mask: usize,
+    /// Optional lock manager (if None, uses global singleton)
+    manager: Option<Arc<GlobalLockManager>>,
 }
 
 impl LocalClient {
-    /// Create new local client
+    /// Create new local client with default shard count
     pub fn new() -> Self {
+        Self::with_shard_count(DEFAULT_GUARD_SHARD_COUNT)
+    }
+
+    /// Create new local client with custom shard count
+    /// Shard count must be a power of 2 for efficient masking
+    pub fn with_shard_count(shard_count: usize) -> Self {
+        assert!(shard_count.is_power_of_two(), "Shard count must be power of 2");
+
+        let guard_storage: Vec<Arc<RwLock<HashMap<LockId, FastLockGuard>>>> =
+            (0..shard_count).map(|_| Arc::new(RwLock::new(HashMap::new()))).collect();
+
         Self {
-            guard_storage: Arc::new(RwLock::new(HashMap::new())),
+            guard_storage,
+            shard_mask: shard_count - 1,
+            manager: None,
         }
     }
 
-    /// Get the global lock manager
+    /// Create new local client with a specific lock manager
+    /// This allows simulating multi-node environments where each node has its own lock backend
+    pub fn with_manager(manager: Arc<GlobalLockManager>) -> Self {
+        Self {
+            guard_storage: (0..DEFAULT_GUARD_SHARD_COUNT)
+                .map(|_| Arc::new(RwLock::new(HashMap::new())))
+                .collect(),
+            shard_mask: DEFAULT_GUARD_SHARD_COUNT - 1,
+            manager: Some(manager),
+        }
+    }
+
+    /// Get the lock manager (injected manager if available, otherwise global singleton)
     pub fn get_lock_manager(&self) -> Arc<GlobalLockManager> {
-        crate::get_global_lock_manager()
+        self.manager.clone().unwrap_or_else(crate::get_global_lock_manager)
+    }
+
+    /// Get the shard index for a given lock ID
+    fn get_shard_index(&self, lock_id: &LockId) -> usize {
+        let mut hasher = std::collections::hash_map::DefaultHasher::new();
+        lock_id.hash(&mut hasher);
+        (hasher.finish() as usize) & self.shard_mask
+    }
+
+    /// Get the shard for a given lock ID
+    fn get_shard(&self, lock_id: &LockId) -> &Arc<RwLock<HashMap<LockId, FastLockGuard>>> {
+        let index = self.get_shard_index(lock_id);
+        &self.guard_storage[index]
     }
 }
 
@@ -49,67 +96,30 @@ impl Default for LocalClient {
 
 #[async_trait::async_trait]
 impl LockClient for LocalClient {
-    async fn acquire_exclusive(&self, request: &LockRequest) -> Result<LockResponse> {
+    async fn acquire_lock(&self, request: &LockRequest) -> Result<LockResponse> {
         let lock_manager = self.get_lock_manager();
-        let lock_request = crate::ObjectLockRequest::new_write("", request.resource.clone(), request.owner.clone())
-            .with_acquire_timeout(request.acquire_timeout);
+
+        let lock_request = match request.lock_type {
+            LockType::Exclusive => crate::ObjectLockRequest::new_write(request.resource.clone(), request.owner.clone())
+                .with_acquire_timeout(request.acquire_timeout),
+            LockType::Shared => crate::ObjectLockRequest::new_read(request.resource.clone(), request.owner.clone())
+                .with_acquire_timeout(request.acquire_timeout),
+        };
 
         match lock_manager.acquire_lock(lock_request).await {
             Ok(guard) => {
-                let lock_id = LockId::new_deterministic(&request.resource);
+                let lock_id = LockId::new_unique(&request.resource);
 
-                // Store guard for later release
-                let mut guards = self.guard_storage.write().await;
-                guards.insert(lock_id.clone(), guard);
+                {
+                    let shard = self.get_shard(&lock_id);
+                    let mut guards = shard.write().await;
+                    guards.insert(lock_id.clone(), guard);
+                }
 
                 let lock_info = LockInfo {
                     id: lock_id,
                     resource: request.resource.clone(),
-                    lock_type: LockType::Exclusive,
-                    status: crate::types::LockStatus::Acquired,
-                    owner: request.owner.clone(),
-                    acquired_at: std::time::SystemTime::now(),
-                    expires_at: std::time::SystemTime::now() + request.ttl,
-                    last_refreshed: std::time::SystemTime::now(),
-                    metadata: request.metadata.clone(),
-                    priority: request.priority,
-                    wait_start_time: None,
-                };
-                Ok(LockResponse::success(lock_info, std::time::Duration::ZERO))
-            }
-            Err(crate::fast_lock::LockResult::Timeout) => {
-                Ok(LockResponse::failure("Lock acquisition timeout", request.acquire_timeout))
-            }
-            Err(crate::fast_lock::LockResult::Conflict {
-                current_owner,
-                current_mode,
-            }) => Ok(LockResponse::failure(
-                format!("Lock conflict: resource held by {current_owner} in {current_mode:?} mode"),
-                std::time::Duration::ZERO,
-            )),
-            Err(crate::fast_lock::LockResult::Acquired) => {
-                unreachable!("Acquired should not be an error")
-            }
-        }
-    }
-
-    async fn acquire_shared(&self, request: &LockRequest) -> Result<LockResponse> {
-        let lock_manager = self.get_lock_manager();
-        let lock_request = crate::ObjectLockRequest::new_read("", request.resource.clone(), request.owner.clone())
-            .with_acquire_timeout(request.acquire_timeout);
-
-        match lock_manager.acquire_lock(lock_request).await {
-            Ok(guard) => {
-                let lock_id = LockId::new_deterministic(&request.resource);
-
-                // Store guard for later release
-                let mut guards = self.guard_storage.write().await;
-                guards.insert(lock_id.clone(), guard);
-
-                let lock_info = LockInfo {
-                    id: lock_id,
-                    resource: request.resource.clone(),
-                    lock_type: LockType::Shared,
+                    lock_type: request.lock_type,
                     status: crate::types::LockStatus::Acquired,
                     owner: request.owner.clone(),
                     acquired_at: std::time::SystemTime::now(),
@@ -138,7 +148,8 @@ impl LockClient for LocalClient {
     }
 
     async fn release(&self, lock_id: &LockId) -> Result<bool> {
-        let mut guards = self.guard_storage.write().await;
+        let shard = self.get_shard(lock_id);
+        let mut guards = shard.write().await;
         if let Some(guard) = guards.remove(lock_id) {
             // Guard automatically releases the lock when dropped
             drop(guard);
@@ -159,7 +170,8 @@ impl LockClient for LocalClient {
     }
 
     async fn check_status(&self, lock_id: &LockId) -> Result<Option<LockInfo>> {
-        let guards = self.guard_storage.read().await;
+        let shard = self.get_shard(lock_id);
+        let guards = shard.read().await;
         if let Some(guard) = guards.get(lock_id) {
             // We have an active guard for this lock
             let lock_type = match guard.mode() {
@@ -200,181 +212,3 @@ impl LockClient for LocalClient {
         true
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::LockType;
-
-    #[tokio::test]
-    async fn test_local_client_acquire_exclusive() {
-        let client = LocalClient::new();
-        let resource_name = format!("test-resource-exclusive-{}", uuid::Uuid::new_v4());
-        let request = LockRequest::new(&resource_name, LockType::Exclusive, "test-owner")
-            .with_acquire_timeout(std::time::Duration::from_secs(30));
-
-        let response = client.acquire_exclusive(&request).await.unwrap();
-        assert!(response.is_success());
-
-        // Clean up
-        if let Some(lock_info) = response.lock_info() {
-            let _ = client.release(&lock_info.id).await;
-        }
-    }
-
-    #[tokio::test]
-    async fn test_local_client_acquire_shared() {
-        let client = LocalClient::new();
-        let resource_name = format!("test-resource-shared-{}", uuid::Uuid::new_v4());
-        let request = LockRequest::new(&resource_name, LockType::Shared, "test-owner")
-            .with_acquire_timeout(std::time::Duration::from_secs(30));
-
-        let response = client.acquire_shared(&request).await.unwrap();
-        assert!(response.is_success());
-
-        // Clean up
-        if let Some(lock_info) = response.lock_info() {
-            let _ = client.release(&lock_info.id).await;
-        }
-    }
-
-    #[tokio::test]
-    async fn test_local_client_release() {
-        let client = LocalClient::new();
-        let resource_name = format!("test-resource-release-{}", uuid::Uuid::new_v4());
-
-        // First acquire a lock
-        let request = LockRequest::new(&resource_name, LockType::Exclusive, "test-owner")
-            .with_acquire_timeout(std::time::Duration::from_secs(30));
-        let response = client.acquire_exclusive(&request).await.unwrap();
-        assert!(response.is_success());
-
-        // Get the lock ID from the response
-        if let Some(lock_info) = response.lock_info() {
-            let result = client.release(&lock_info.id).await.unwrap();
-            assert!(result);
-        } else {
-            panic!("No lock info in response");
-        }
-    }
-
-    #[tokio::test]
-    async fn test_local_client_is_local() {
-        let client = LocalClient::new();
-        assert!(client.is_local().await);
-    }
-
-    #[tokio::test]
-    async fn test_local_client_read_write_lock_exclusion() {
-        let client = LocalClient::new();
-        let resource_name = format!("test-resource-exclusion-{}", uuid::Uuid::new_v4());
-
-        // First, acquire an exclusive lock
-        let exclusive_request = LockRequest::new(&resource_name, LockType::Exclusive, "exclusive-owner")
-            .with_acquire_timeout(std::time::Duration::from_millis(10));
-        let exclusive_response = client.acquire_exclusive(&exclusive_request).await.unwrap();
-        assert!(exclusive_response.is_success());
-
-        // Try to acquire a shared lock on the same resource - should fail
-        let shared_request = LockRequest::new(&resource_name, LockType::Shared, "shared-owner")
-            .with_acquire_timeout(std::time::Duration::from_millis(10));
-        let shared_response = client.acquire_shared(&shared_request).await.unwrap();
-        assert!(!shared_response.is_success(), "Shared lock should fail when exclusive lock exists");
-
-        // Clean up exclusive lock
-        if let Some(exclusive_info) = exclusive_response.lock_info() {
-            let _ = client.release(&exclusive_info.id).await;
-        }
-
-        // Now shared lock should succeed
-        let shared_request2 = LockRequest::new(&resource_name, LockType::Shared, "shared-owner")
-            .with_acquire_timeout(std::time::Duration::from_millis(10));
-        let shared_response2 = client.acquire_shared(&shared_request2).await.unwrap();
-        assert!(
-            shared_response2.is_success(),
-            "Shared lock should succeed after exclusive lock is released"
-        );
-
-        // Clean up
-        if let Some(shared_info) = shared_response2.lock_info() {
-            let _ = client.release(&shared_info.id).await;
-        }
-    }
-
-    #[tokio::test]
-    async fn test_local_client_read_write_lock_distinction() {
-        let client = LocalClient::new();
-        let resource_name = format!("test-resource-rw-{}", uuid::Uuid::new_v4());
-
-        // Test exclusive lock
-        let exclusive_request = LockRequest::new(&resource_name, LockType::Exclusive, "exclusive-owner")
-            .with_acquire_timeout(std::time::Duration::from_secs(30));
-        let exclusive_response = client.acquire_exclusive(&exclusive_request).await.unwrap();
-        assert!(exclusive_response.is_success());
-
-        if let Some(exclusive_info) = exclusive_response.lock_info() {
-            assert_eq!(exclusive_info.lock_type, LockType::Exclusive);
-
-            // Check status should return correct lock type
-            let status = client.check_status(&exclusive_info.id).await.unwrap();
-            assert!(status.is_some());
-            assert_eq!(status.unwrap().lock_type, LockType::Exclusive);
-
-            // Release exclusive lock
-            let result = client.release(&exclusive_info.id).await.unwrap();
-            assert!(result);
-        }
-
-        // Test shared lock
-        let shared_request = LockRequest::new(&resource_name, LockType::Shared, "shared-owner")
-            .with_acquire_timeout(std::time::Duration::from_secs(30));
-        let shared_response = client.acquire_shared(&shared_request).await.unwrap();
-        assert!(shared_response.is_success());
-
-        if let Some(shared_info) = shared_response.lock_info() {
-            assert_eq!(shared_info.lock_type, LockType::Shared);
-
-            // Check status should return correct lock type
-            let status = client.check_status(&shared_info.id).await.unwrap();
-            assert!(status.is_some());
-            assert_eq!(status.unwrap().lock_type, LockType::Shared);
-
-            // Release shared lock
-            let result = client.release(&shared_info.id).await.unwrap();
-            assert!(result);
-        }
-    }
-
-    #[tokio::test]
-    async fn test_multiple_local_clients_exclusive_mutex() {
-        let client1 = LocalClient::new();
-        let client2 = LocalClient::new();
-        let resource_name = format!("test-multi-client-mutex-{}", uuid::Uuid::new_v4());
-
-        // client1 acquire exclusive lock
-        let req1 = LockRequest::new(&resource_name, LockType::Exclusive, "owner1")
-            .with_acquire_timeout(std::time::Duration::from_millis(50));
-        let resp1 = client1.acquire_exclusive(&req1).await.unwrap();
-        assert!(resp1.is_success(), "client1 should acquire exclusive lock");
-
-        // client2 try to acquire exclusive lock, should fail
-        let req2 = LockRequest::new(&resource_name, LockType::Exclusive, "owner2")
-            .with_acquire_timeout(std::time::Duration::from_millis(50));
-        let resp2 = client2.acquire_exclusive(&req2).await.unwrap();
-        assert!(!resp2.is_success(), "client2 should not acquire exclusive lock while client1 holds it");
-
-        // client1 release lock
-        if let Some(lock_info) = resp1.lock_info() {
-            let _ = client1.release(&lock_info.id).await;
-        }
-
-        // client2 try again, should succeed
-        let resp3 = client2.acquire_exclusive(&req2).await.unwrap();
-        assert!(resp3.is_success(), "client2 should acquire exclusive lock after client1 releases it");
-
-        // clean up
-        if let Some(lock_info) = resp3.lock_info() {
-            let _ = client2.release(&lock_info.id).await;
-        }
-    }
-}

crates/lock/src/client/mod.rs

@@ -15,26 +15,15 @@
 pub mod local;
 // pub mod remote;
 
-use crate::{LockId, LockInfo, LockRequest, LockResponse, LockStats, LockType, Result};
+use crate::{LockId, LockInfo, LockRequest, LockResponse, LockStats, Result};
 use async_trait::async_trait;
 use std::sync::Arc;
 
 /// Lock client trait
 #[async_trait]
 pub trait LockClient: Send + Sync + std::fmt::Debug {
-    /// Acquire exclusive lock
-    async fn acquire_exclusive(&self, request: &LockRequest) -> Result<LockResponse>;
-
-    /// Acquire shared lock
-    async fn acquire_shared(&self, request: &LockRequest) -> Result<LockResponse>;
-
     /// Acquire lock (generic method)
-    async fn acquire_lock(&self, request: &LockRequest) -> Result<LockResponse> {
-        match request.lock_type {
-            LockType::Exclusive => self.acquire_exclusive(request).await,
-            LockType::Shared => self.acquire_shared(request).await,
-        }
-    }
+    async fn acquire_lock(&self, request: &LockRequest) -> Result<LockResponse>;
 
     /// Release lock
     async fn release(&self, lock_id: &LockId) -> Result<bool>;
@@ -75,36 +64,3 @@ impl ClientFactory {
     //     Arc::new(remote::RemoteClient::new(endpoint))
     // }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::LockType;
-
-    #[tokio::test]
-    async fn test_local_client_basic_operations() {
-        let client = ClientFactory::create_local();
-
-        let request = LockRequest::new("test-resource", LockType::Exclusive, "test-owner");
-
-        // Test lock acquisition
-        let response = client.acquire_exclusive(&request).await;
-        assert!(response.is_ok());
-
-        if let Ok(response) = response
-            && response.success
-        {
-            let lock_info = response.lock_info.unwrap();
-
-            // Test status check
-            let status = client.check_status(&lock_info.id).await;
-            assert!(status.is_ok());
-            assert!(status.unwrap().is_some());
-
-            // Test lock release
-            let released = client.release(&lock_info.id).await;
-            assert!(released.is_ok());
-            assert!(released.unwrap());
-        }
-    }
-}