charlesdong1991 commented on code in PR #369:
URL: https://github.com/apache/fluss-rust/pull/369#discussion_r2849588902


##########
crates/fluss/src/client/connection.rs:
##########
@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::client::WriterClient;
+use crate::client::{WriterClient, admin};

Review Comment:
   FlussAdmin is imported already below, by adding `admin`, it will it can be 
reached in 2 ways. i think we should have a consistent way for module import.



##########
crates/fluss/src/client/connection.rs:
##########
@@ -60,7 +62,27 @@ impl FlussConnection {
     }
 
     pub async fn get_admin(&self) -> Result<FlussAdmin> {
-        FlussAdmin::new(self.network_connects.clone(), 
self.metadata.clone()).await
+        // 1. Fast path: Attempt to acquire a read lock to check if the admin 
already exists.
+        if let Some(admin) = self.admin_client.read().as_ref() {
+            return Ok(admin.as_ref().clone());
+        }
+
+        // 2. Initialize the admin outside the guard lock 
+        let new_admin = 
Arc::new(admin::FlussAdmin::new(self.network_connects.clone(), 
self.metadata.clone()).await?);
+        
+        // 3. Slow path: Acquire the write lock.
+        let mut admin_guard = self.admin_client.write();
+
+        // 4. Double-check: Another thread might have initialized the admin
+        // while this thread was waiting for the write lock.
+        if let Some(admin) = admin_guard.as_ref() {
+            return Ok(admin.as_ref().clone());
+        }
+
+        // 5. Store and return the newly created admin.
+        let result = new_admin.as_ref().clone();
+        *admin_guard = Some(new_admin);
+        Ok(result)

Review Comment:
   +1



##########
crates/fluss/src/client/connection.rs:
##########
@@ -60,7 +62,27 @@ impl FlussConnection {
     }
 
     pub async fn get_admin(&self) -> Result<FlussAdmin> {
-        FlussAdmin::new(self.network_connects.clone(), 
self.metadata.clone()).await
+        // 1. Fast path: Attempt to acquire a read lock to check if the admin 
already exists.
+        if let Some(admin) = self.admin_client.read().as_ref() {
+            return Ok(admin.as_ref().clone());
+        }
+
+        // 2. Initialize the admin outside the guard lock 
+        let new_admin = 
Arc::new(admin::FlussAdmin::new(self.network_connects.clone(), 
self.metadata.clone()).await?);
+        
+        // 3. Slow path: Acquire the write lock.
+        let mut admin_guard = self.admin_client.write();
+

Review Comment:
   +1



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to