CORE: Persist Client Name Across Reconnections for CLIENT SETNAME Command (#4841)

* persist client name across reconnections for CLIENT SETNAME command

- Add client name update methods to Client, StandaloneClient, and cluster clients
- Intercept CLIENT SETNAME commands to update stored connection info
- Ensure client name persists through automatic reconnections
- Add comprehensive tests for client name persistence behavior
- Support both standalone and cluster connection modes

Signed-off-by: affonsov <[email protected]>

* added changelog

Signed-off-by: affonsov <[email protected]>

* clean up test

Signed-off-by: affonsov <[email protected]>

* refactored fucntion to not return value
clean up the test

Signed-off-by: affonsov <[email protected]>

* fix lint

Signed-off-by: affonsov <[email protected]>

* fix lint

Signed-off-by: affonsov <[email protected]>

---------

Signed-off-by: affonsov <[email protected]>

Author: affonsov

CHANGELOG.md

@@ -19,6 +19,7 @@
 
 * CORE: Fix SELECT Command Database Persistence Across Reconnections ([#4764](https://github.com/valkey-io/valkey-glide/issues/#4764))
 * Rust: Updates the `install-rust-and-protoc` action to explicitly include the `rustfmt` and `clippy` components. ([#4816](https://github.com/valkey-io/valkey-glide/issues/4816))
+* CORE: Persist Client Name Across Reconnections for CLIENT SETNAME Command. ([#4841](https://github.com/valkey-io/valkey-glide/pull/4841))
 
 ## 2.1
 

glide-core/redis-rs/redis/src/client.rs

@@ -583,6 +583,11 @@ impl Client {
     pub fn update_database(&mut self, database_id: i64) {
         self.connection_info.redis.db = database_id;
     }
+
+    /// Updates the client_name in connection_info.
+    pub fn update_client_name(&mut self, client_name: Option<String>) {
+        self.connection_info.redis.client_name = client_name;
+    }
 }
 
 #[cfg(feature = "aio")]

glide-core/redis-rs/redis/src/cluster_async/mod.rs

@@ -338,6 +338,15 @@ where
             .await
     }
 
+    /// Update the name used for all cluster connections
+    pub async fn update_connection_client_name(
+        &mut self,
+        client_name: Option<String>,
+    ) -> RedisResult<Value> {
+        self.route_operation_request(Operation::UpdateConnectionClientName(client_name))
+            .await
+    }
+
     /// Get the username used to authenticate with all cluster servers
     pub async fn get_username(&mut self) -> RedisResult<Value> {
         self.route_operation_request(Operation::GetUsername).await
@@ -662,6 +671,7 @@ enum CmdArg<C> {
 enum Operation {
     UpdateConnectionPassword(Option<String>),
     UpdateConnectionDatabase(i64),
+    UpdateConnectionClientName(Option<String>),
     GetUsername,
 }
 
@@ -2604,6 +2614,11 @@ where
                         .expect(MUTEX_WRITE_ERR);
                     Ok(Response::Single(Value::Okay))
                 }
+                Operation::UpdateConnectionClientName(client_name) => {
+                    core.set_cluster_param(|params| params.client_name = client_name)
+                        .expect(MUTEX_WRITE_ERR);
+                    Ok(Response::Single(Value::Okay))
+                }
                 Operation::GetUsername => {
                     let username = match core
                         .get_cluster_param(|params| params.username.clone())

glide-core/src/client/mod.rs

@@ -399,6 +399,58 @@ impl Client {
         }
     }
 
+    /// Checks if the given command is a CLIENT SETNAME command.
+    /// Returns true if the command is "CLIENT SETNAME", false otherwise.
+    fn is_client_set_name_command(&self, cmd: &Cmd) -> bool {
+        // Check if the command is "CLIENT SETNAME"
+        cmd.command()
+            .is_some_and(|bytes| bytes == b"CLIENT SETNAME")
+    }
+
+    /// Extracts the client name from a CLIENT SETNAME command.
+    /// Parses the client name argument from the CLIENT SETNAME command.
+    /// Returns None if the argument is missing or invalid.
+    fn extract_client_name_from_client_set_name(&self, cmd: &Cmd) -> Option<String> {
+        // For redis::cmd("CLIENT").arg("SETNAME").arg("name")
+        // the client name is at arg_idx(2) (after "SETNAME")
+        cmd.arg_idx(2).and_then(|name_bytes| {
+            std::str::from_utf8(name_bytes)
+                .ok()
+                .map(|name_str| name_str.to_string())
+        })
+    }
+
+    /// Handles CLIENT SETNAME command processing after successful execution.
+    /// Updates connection name state for standalone, cluster, and lazy clients.
+    async fn handle_client_set_name_command(&mut self, cmd: &Cmd) -> RedisResult<()> {
+        // Extract client name from the CLIENT SETNAME command
+        let client_name = self.extract_client_name_from_client_set_name(cmd);
+
+        // Update client name state for all client types
+        self.update_stored_client_name(client_name).await?;
+        Ok(())
+    }
+
+    /// Updates the stored client name for different client types.
+    /// Handles standalone, cluster, and lazy clients appropriately.
+    /// Ensures thread-safe updates using existing synchronization mechanisms.
+    async fn update_stored_client_name(&self, client_name: Option<String>) -> RedisResult<()> {
+        let mut guard = self.internal_client.write().await;
+        match &mut *guard {
+            ClientWrapper::Standalone(client) => {
+                client.update_connection_client_name(client_name).await?;
+                Ok(())
+            }
+            ClientWrapper::Cluster { client } => {
+                // Update cluster connection database configuration
+                client.update_connection_client_name(client_name).await?;
+                Ok(())
+            }
+            ClientWrapper::Lazy(_) => {
+                unreachable!("Lazy client should have been initialized")
+            }
+        }
+    }
     async fn get_or_initialize_client(&self) -> RedisResult<ClientWrapper> {
         {
             let guard = self.internal_client.read().await;
@@ -496,6 +548,12 @@ impl Client {
             })
             .await?;
 
+            // Intercept CLIENT SETNAME commands after regular processing
+            // Only handle CLIENT SETNAME commands if they executed successfully (no error)
+            if self.is_client_set_name_command(cmd) {
+                self.handle_client_set_name_command(cmd).await?;
+            }
+
             // Intercept SELECT commands after regular processing
             // Only handle SELECT commands if they executed successfully (no error)
             if self.is_select_command(cmd) {
@@ -1522,4 +1580,54 @@ mod tests {
             inflight_requests_allowed: Arc::new(AtomicIsize::new(1000)),
         }
     }
+
+    #[test]
+    fn test_is_client_set_name_command() {
+        // Create a mock client for testing
+        let client = create_test_client();
+
+        // Test valid CLIENT SETNAME command
+        let mut cmd = Cmd::new();
+        cmd.arg("CLIENT").arg("SETNAME").arg("test_client");
+        assert!(client.is_client_set_name_command(&cmd));
+
+        // Test CLIENT SETNAME with different case (should work due to case-insensitive comparison)
+        let mut cmd = Cmd::new();
+        cmd.arg("client").arg("setname").arg("test_client");
+        assert!(client.is_client_set_name_command(&cmd));
+
+        // Test CLIENT command without SETNAME
+        let mut cmd = Cmd::new();
+        cmd.arg("CLIENT").arg("INFO");
+        assert!(!client.is_client_set_name_command(&cmd));
+
+        // Test non-CLIENT command
+        let mut cmd = Cmd::new();
+        cmd.arg("SET").arg("key").arg("value");
+        assert!(!client.is_client_set_name_command(&cmd));
+
+        // Test CLIENT SETNAME without client name argument
+        let mut cmd = Cmd::new();
+        cmd.arg("CLIENT").arg("SETNAME");
+        assert!(client.is_client_set_name_command(&cmd));
+
+        // Test CLIENT only
+        let mut cmd = Cmd::new();
+        cmd.arg("CLIENT");
+        assert!(!client.is_client_set_name_command(&cmd));
+    }
+
+    #[test]
+    fn test_extract_client_name_from_client_set_name() {
+        // Test detection of valid CLIENT SETNAME commands
+        let client = create_test_client();
+
+        // Test uppercase CLIENT SETNAME command
+        let mut cmd = Cmd::new();
+        cmd.arg("CLIENT").arg("SETNAME").arg("test_name");
+        assert_eq!(
+            client.extract_client_name_from_client_set_name(&cmd),
+            Some("test_name".to_string())
+        );
+    }
 }

glide-core/src/client/reconnecting_connection.rs

@@ -416,6 +416,17 @@ impl ReconnectingConnection {
         client.update_database(new_database_id);
     }
 
+    /// Updates the client name that's saved inside connection_info, that will be used in case of disconnection from the server.
+    pub(crate) fn update_connection_client_name(&self, new_client_name: Option<String>) {
+        let mut client = self
+            .inner
+            .backend
+            .connection_info
+            .write()
+            .expect(WRITE_LOCK_ERR);
+        client.update_client_name(new_client_name);
+    }
+
     /// Returns the username if one was configured during client creation. Otherwise, returns None.
     pub(crate) fn get_username(&self) -> Option<String> {
         let client = self.inner.backend.get_backend_client();

glide-core/src/client/standalone_client.rs

@@ -632,6 +632,18 @@ impl StandaloneClient {
         Ok(Value::Okay)
     }
 
+    /// Update the client_name used to create the connection.
+    pub async fn update_connection_client_name(
+        &self,
+        new_client_name: Option<String>,
+    ) -> RedisResult<Value> {
+        for node in self.inner.nodes.iter() {
+            node.update_connection_client_name(new_client_name.clone());
+        }
+
+        Ok(Value::Okay)
+    }
+
     /// Retrieve the username used to authenticate with the server.
     pub fn get_username(&self) -> Option<String> {
         // All nodes in the client should have the same username configured, thus any connection would work here.

glide-core/tests/test_client.rs

@@ -1667,4 +1667,138 @@ pub(crate) mod shared_client_tests {
             }
         });
     }
+
+    #[rstest]
+    #[serial_test::serial]
+    #[timeout(SHORT_CLUSTER_TEST_TIMEOUT)]
+    /// Test that verifies the client maintains the correct client name after an automatic reconnection
+    /// when the client name was changed using the CLIENT SETNAME command.
+    /// This test:
+    /// 1. Creates a client with a name "1stName"
+    /// 2. Uses CLIENT SETNAME command to change to "2ndName"
+    /// 3. Verifies the connection is now with the "2ndName"
+    /// 4. Simulates a connection drop by killing the connection
+    /// 5. Sends another command which either:
+    ///    - Fails due to the dropped connection, then retries and verifies reconnection has the name "2ndName" set
+    ///    - Succeeds with a new client ID (indicating reconnection) and verifies still has the name "2ndName" set
+    /// This ensures that client name via CLIENT SETNAME command persists across reconnections.
+    fn test_client_set_name_command_persistence_after_reconnection(
+        #[values(false, true)] use_cluster: bool,
+    ) {
+        block_on_all(async move {
+            let mut test_basics = setup_test_basics(
+                use_cluster,
+                TestConfiguration {
+                    client_name: Some("1stName".to_string()),
+                    shared_server: true,
+                    ..Default::default()
+                },
+            )
+            .await;
+
+            let mut client_info_cmd = redis::Cmd::new();
+            client_info_cmd.arg("CLIENT").arg("INFO");
+
+            let mut client_setname_cmd = redis::Cmd::new();
+            client_setname_cmd
+                .arg("Client")
+                .arg("SETNAME")
+                .arg("2ndName");
+
+            // Verify initial connection client name
+            let initial_client_info_response = test_basics
+                .client
+                .send_command(&client_info_cmd, None)
+                .await
+                .unwrap();
+
+            let initial_client_info = match initial_client_info_response {
+                Value::BulkString(bytes) => String::from_utf8_lossy(&bytes).to_string(),
+                Value::VerbatimString { text, .. } => text,
+                _ => panic!(
+                    "Unexpected CLIENT INFO response type: {:?}",
+                    initial_client_info_response
+                ),
+            };
+            assert!(initial_client_info.contains("name=1stName"));
+
+            // Extract initial client ID
+            let initial_client_id = utilities::extract_client_id(&initial_client_info)
+                .expect("Failed to extract initial client ID");
+
+            // Execute CLIENT SETNAME command to change to 2ndName
+            let client_setname_result = test_basics
+                .client
+                .send_command(&client_setname_cmd, None)
+                .await
+                .unwrap();
+            assert_eq!(client_setname_result, Value::Okay);
+
+            // Verify we're now on 2ndName
+            let post_client_setname_client_info_response = test_basics
+                .client
+                .send_command(&client_info_cmd, None)
+                .await
+                .unwrap();
+
+            let post_client_setname_client_info = match post_client_setname_client_info_response {
+                Value::BulkString(bytes) => String::from_utf8_lossy(&bytes).to_string(),
+                Value::VerbatimString { text, .. } => text,
+                _ => panic!(
+                    "Unexpected CLIENT INFO response type: {:?}",
+                    post_client_setname_client_info_response
+                ),
+            };
+            assert!(post_client_setname_client_info.contains("name=2ndName"));
+
+            // Kill the connection to simulate a network drop
+            kill_connection(&mut test_basics.client).await;
+
+            // Try to send another command - this should trigger reconnection
+            let res = test_basics
+                .client
+                .send_command(&client_info_cmd, None)
+                .await;
+            match res {
+                Err(err) => {
+                    // Connection was dropped as expected
+                    assert!(
+                        err.is_connection_dropped() || err.is_timeout(),
+                        "Expected connection dropped or timeout error, got: {err:?}",
+                    );
+                    // Retry and verify we're still on name 2ndName after reconnection
+                    let client_info = repeat_try_create(|| async {
+                        let mut client = test_basics.client.clone();
+                        let response = client.send_command(&client_info_cmd, None).await.ok()?;
+                        match response {
+                            Value::BulkString(bytes) => {
+                                Some(String::from_utf8_lossy(&bytes).to_string())
+                            }
+                            Value::VerbatimString { text, .. } => Some(text),
+                            _ => None,
+                        }
+                    })
+                    .await;
+                    assert!(client_info.contains("name=2ndName"));
+                }
+                Ok(response) => {
+                    // Command succeeded, extract new client ID and compare
+                    let new_client_info = match response {
+                        Value::BulkString(bytes) => String::from_utf8_lossy(&bytes).to_string(),
+                        Value::VerbatimString { text, .. } => text,
+                        _ => panic!("Unexpected CLIENT INFO response type: {:?}", response),
+                    };
+                    let new_client_id = utilities::extract_client_id(&new_client_info)
+                        .expect("Failed to extract new client ID");
+                    assert_ne!(
+                        initial_client_id, new_client_id,
+                        "Client ID should change after reconnection if command succeeds"
+                    );
+                    // Check that the client name is still 2ndName (from CLIENT SETNAME command)
+                    println!("{}", new_client_info);
+                    assert!(new_client_info.contains("name=2ndName"));
+                }
+            }
+        });
+    }
 }