fix(lifecycle): prevent eager date-expiry deletion on config update (#2708)

crates/ecstore/src/rpc/client.rs

@@ -12,11 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::disk::error::{DiskError, Error as DiskErrorType};
 use crate::rpc::{TONIC_RPC_PREFIX, gen_signature_headers};
 use http::Method;
 use rustfs_common::GLOBAL_CONN_MAP;
 use rustfs_protos::{create_new_channel, proto_gen::node_service::node_service_client::NodeServiceClient};
-use std::error::Error;
+use std::{error::Error, io::ErrorKind};
 use tonic::{service::interceptor::InterceptedService, transport::Channel};
 use tracing::debug;
 
@@ -51,6 +52,46 @@ pub async fn node_service_time_out_client_no_auth(
     node_service_time_out_client(addr, TonicInterceptor::NoOp(NoOpInterceptor)).await
 }
 
+pub(crate) fn is_network_like_disk_error(err: &DiskErrorType) -> bool {
+    match err {
+        DiskError::Timeout => true,
+        DiskError::Io(io_err) => {
+            if matches!(
+                io_err.kind(),
+                ErrorKind::TimedOut
+                    | ErrorKind::ConnectionRefused
+                    | ErrorKind::ConnectionReset
+                    | ErrorKind::BrokenPipe
+                    | ErrorKind::NotConnected
+                    | ErrorKind::ConnectionAborted
+                    | ErrorKind::UnexpectedEof
+            ) {
+                return true;
+            }
+
+            let message = io_err.to_string().to_ascii_lowercase();
+            [
+                "transport error",
+                "unavailable",
+                "error trying to connect",
+                "connection refused",
+                "connection reset",
+                "broken pipe",
+                "not connected",
+                "unexpected eof",
+                "timed out",
+                "deadline has elapsed",
+                "connection closed",
+                "connection aborted",
+                "tcp connect error",
+            ]
+            .iter()
+            .any(|needle| message.contains(needle))
+        }
+        _ => false,
+    }
+}
+
 pub struct TonicSignatureInterceptor;
 
 impl tonic::service::Interceptor for TonicSignatureInterceptor {

crates/ecstore/src/rpc/peer_s3_client.rs

@@ -18,13 +18,15 @@ use crate::disk::error::{Error, Result};
 use crate::disk::error_reduce::{BUCKET_OP_IGNORED_ERRS, is_all_buckets_not_found, reduce_write_quorum_errs};
 use crate::disk::{DiskAPI, DiskStore, disk_store::get_max_timeout_duration};
 use crate::global::GLOBAL_LOCAL_DISK_MAP;
-use crate::rpc::client::{TonicInterceptor, gen_tonic_signature_interceptor, node_service_time_out_client};
+use crate::rpc::client::{
+    TonicInterceptor, gen_tonic_signature_interceptor, is_network_like_disk_error, node_service_time_out_client,
+};
 use crate::store::all_local_disk;
 use crate::store_utils::is_reserved_or_invalid_bucket;
 use crate::{
     disk::{
         self, VolumeInfo,
-        disk_store::{CHECK_EVERY, CHECK_TIMEOUT_DURATION, DiskHealthTracker},
+        disk_store::{DiskHealthTracker, get_drive_active_check_interval, get_drive_active_check_timeout},
     },
     endpoints::{EndpointServerPools, Node},
     store_api::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions},
@@ -613,7 +615,7 @@ impl RemotePeerS3Client {
 
     /// Monitor remote peer health periodically
     async fn monitor_remote_peer_health(addr: String, health: Arc<DiskHealthTracker>, cancel_token: CancellationToken) {
-        let mut interval = time::interval(CHECK_EVERY);
+        let mut interval = time::interval(get_drive_active_check_interval());
 
         loop {
             tokio::select! {
@@ -682,7 +684,7 @@ impl RemotePeerS3Client {
         let port = url.port_or_known_default().unwrap_or(80);
 
         // Try to establish TCP connection
-        match timeout(CHECK_TIMEOUT_DURATION, TcpStream::connect((host, port))).await {
+        match timeout(get_drive_active_check_timeout(), TcpStream::connect((host, port))).await {
             Ok(Ok(_)) => Ok(()),
             _ => Err(Error::other(format!("Cannot connect to {host}:{port}"))),
         }
@@ -717,16 +719,39 @@ impl RemotePeerS3Client {
                     self.health.log_success();
                 }
                 self.health.decrement_waiting();
+                if let Err(err) = &operation_result
+                    && is_network_like_disk_error(err)
+                {
+                    self.mark_faulty_and_start_recovery("operation_network_error").await;
+                }
                 operation_result
             }
             Err(_) => {
                 // Timeout occurred, mark peer as potentially faulty
                 self.health.decrement_waiting();
+                self.mark_faulty_and_start_recovery("operation_timeout").await;
                 warn!("Remote peer operation timeout after {:?}", timeout_duration);
                 Err(Error::other(format!("Remote peer operation timeout after {timeout_duration:?}")))
             }
         }
     }
+
+    async fn mark_faulty_and_start_recovery(&self, reason: &'static str) {
+        if self.health.swap_ok_to_faulty() {
+            warn!(
+                addr = %self.addr,
+                reason,
+                "Remote peer marked faulty after network failure"
+            );
+
+            let health = Arc::clone(&self.health);
+            let cancel_token = self.cancel_token.clone();
+            let addr = self.addr.clone();
+            tokio::spawn(async move {
+                Self::monitor_remote_peer_recovery(addr, health, cancel_token).await;
+            });
+        }
+    }
 }
 
 #[async_trait]
@@ -1001,3 +1026,69 @@ pub async fn heal_bucket_local(bucket: &str, opts: &HealOpts) -> Result<HealResu
 async fn clone_drives() -> Vec<Option<DiskStore>> {
     GLOBAL_LOCAL_DISK_MAP.read().await.values().cloned().collect::<Vec<_>>()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn test_remote_peer(addr: &str) -> RemotePeerS3Client {
+        let node = Node {
+            url: url::Url::parse(addr).expect("test peer URL should parse"),
+            pools: vec![0],
+            is_local: false,
+            grid_host: addr.to_string(),
+        };
+
+        RemotePeerS3Client {
+            node: Some(node),
+            pools: Some(vec![0]),
+            addr: addr.to_string(),
+            health: Arc::new(DiskHealthTracker::new()),
+            cancel_token: CancellationToken::new(),
+        }
+    }
+
+    #[tokio::test]
+    async fn test_execute_with_timeout_marks_remote_peer_faulty_on_network_like_error() {
+        let client = test_remote_peer("http://peer-network-error:9000");
+
+        let err = client
+            .execute_with_timeout(
+                || async {
+                    Err::<(), Error>(DiskError::Io(std::io::Error::new(
+                        std::io::ErrorKind::ConnectionRefused,
+                        "connection refused",
+                    )))
+                },
+                Duration::from_secs(1),
+            )
+            .await
+            .expect_err("network-like error should fail");
+
+        assert_eq!(
+            match &err {
+                DiskError::Io(io_err) => io_err.kind(),
+                other => panic!("expected io network error, got {other:?}"),
+            },
+            std::io::ErrorKind::ConnectionRefused
+        );
+        assert!(client.health.is_faulty(), "network-like errors should mark remote peer faulty");
+
+        client.cancel_token.cancel();
+    }
+
+    #[tokio::test]
+    async fn test_execute_with_timeout_keeps_remote_peer_online_for_business_error() {
+        let client = test_remote_peer("http://peer-business-error:9000");
+
+        let err = client
+            .execute_with_timeout(|| async { Err::<(), Error>(DiskError::FileNotFound) }, Duration::from_secs(1))
+            .await
+            .expect_err("business error should fail");
+
+        assert_eq!(err, DiskError::FileNotFound);
+        assert!(!client.health.is_faulty(), "business errors should not mark remote peer faulty");
+
+        client.cancel_token.cancel();
+    }
+}

crates/ecstore/src/rpc/remote_disk.rs

@@ -16,15 +16,17 @@ use crate::disk::{
     CheckPartsResp, DeleteOptions, DiskAPI, DiskInfo, DiskInfoOptions, DiskLocation, DiskOption, FileInfoVersions, FileReader,
     FileWriter, ReadMultipleReq, ReadMultipleResp, ReadOptions, RenameDataResp, UpdateMetadataOpts, VolumeInfo, WalkDirOptions,
     disk_store::{
-        CHECK_EVERY, CHECK_TIMEOUT_DURATION, DEFAULT_RUSTFS_DRIVE_ACTIVE_MONITORING, ENV_RUSTFS_DRIVE_ACTIVE_MONITORING,
-        SKIP_IF_SUCCESS_BEFORE, get_drive_disk_info_timeout, get_drive_list_dir_timeout, get_drive_metadata_timeout,
-        get_drive_walkdir_stall_timeout, get_drive_walkdir_timeout, get_max_timeout_duration,
+        DEFAULT_RUSTFS_DRIVE_ACTIVE_MONITORING, ENV_RUSTFS_DRIVE_ACTIVE_MONITORING, SKIP_IF_SUCCESS_BEFORE,
+        get_drive_active_check_interval, get_drive_active_check_timeout, get_drive_disk_info_timeout, get_drive_list_dir_timeout,
+        get_drive_metadata_timeout, get_drive_walkdir_stall_timeout, get_drive_walkdir_timeout, get_max_timeout_duration,
     },
     endpoint::Endpoint,
     health_state::{RuntimeDriveHealthState, get_drive_returning_probe_interval, record_drive_runtime_state},
 };
 use crate::disk::{disk_store::DiskHealthTracker, error::DiskError, local::ScanGuard};
-use crate::rpc::client::{TonicInterceptor, gen_tonic_signature_interceptor, node_service_time_out_client};
+use crate::rpc::client::{
+    TonicInterceptor, gen_tonic_signature_interceptor, is_network_like_disk_error, node_service_time_out_client,
+};
 use crate::set_disk::DEFAULT_READ_BUFFER_SIZE;
 use crate::{
     disk::error::{Error, Result},
@@ -47,7 +49,7 @@ use rustfs_protos::proto_gen::node_service::{
 use rustfs_rio::{HttpReader, HttpWriter};
 use serde::{Serialize, de::DeserializeOwned};
 use std::{
-    io::{Cursor, ErrorKind},
+    io::Cursor,
     path::PathBuf,
     sync::{
         Arc,
@@ -176,7 +178,7 @@ impl RemoteDisk {
         health: Arc<DiskHealthTracker>,
         cancel_token: CancellationToken,
     ) {
-        let mut interval = time::interval(CHECK_EVERY);
+        let mut interval = time::interval(get_drive_active_check_interval());
 
         // Perform basic connectivity check
         if Self::perform_connectivity_check(&addr).await.is_err() && health.mark_offline(&endpoint, "connectivity_probe_failed") {
@@ -281,7 +283,7 @@ impl RemoteDisk {
         let port = url.port_or_known_default().unwrap_or(80);
 
         // Try to establish TCP connection
-        match timeout(CHECK_TIMEOUT_DURATION, TcpStream::connect((host, port))).await {
+        match timeout(get_drive_active_check_timeout(), TcpStream::connect((host, port))).await {
             Ok(Ok(stream)) => {
                 drop(stream);
                 Ok(())
@@ -334,10 +336,10 @@ impl RemoteDisk {
                 }
                 self.health.decrement_waiting();
                 if let Err(err) = &operation_result
-                    && Self::is_timeout_like_error(err)
+                    && is_network_like_disk_error(err)
                 {
                     counter!(
-                        "rustfs_drive_op_timeout_total",
+                        "rustfs_drive_op_network_error_total",
                         "endpoint" => self.endpoint.to_string(),
                         "op" => op.to_string()
                     )
@@ -347,9 +349,9 @@ impl RemoteDisk {
                         addr = %self.addr,
                         op,
                         timeout_ms = timeout_duration.as_millis(),
-                        "Remote disk operation returned a timeout-like error"
+                        "Remote disk operation returned a network-like error"
                     );
-                    self.mark_faulty_and_evict("operation_timeout_error").await;
+                    self.mark_faulty_and_evict("operation_network_error").await;
                 }
                 operation_result
             }
@@ -375,10 +377,6 @@ impl RemoteDisk {
         }
     }
 
-    fn is_timeout_like_error(err: &Error) -> bool {
-        matches!(err, DiskError::Timeout) || matches!(err, DiskError::Io(io_err) if io_err.kind() == ErrorKind::TimedOut)
-    }
-
     async fn mark_faulty_and_evict(&self, reason: &'static str) {
         if self.health.mark_offline(&self.endpoint, reason) {
             self.spawn_recovery_monitor_if_needed();
@@ -2073,6 +2071,96 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn test_execute_with_timeout_marks_faulty_on_network_like_error() {
+        let addr = "http://127.0.0.1:59993".to_string();
+        let url = url::Url::parse(&format!("{addr}/data")).unwrap();
+        let endpoint = Endpoint {
+            url,
+            is_local: false,
+            pool_idx: 0,
+            set_idx: 0,
+            disk_idx: 0,
+        };
+
+        let remote_disk = RemoteDisk::new(
+            &endpoint,
+            &DiskOption {
+                cleanup: false,
+                health_check: false,
+            },
+        )
+        .await
+        .unwrap();
+
+        let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy();
+        GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel);
+
+        let err = remote_disk
+            .execute_with_timeout(
+                || async {
+                    Err::<(), Error>(DiskError::Io(std::io::Error::new(
+                        std::io::ErrorKind::ConnectionRefused,
+                        "connection refused",
+                    )))
+                },
+                Duration::from_secs(1),
+            )
+            .await
+            .expect_err("network-like operation error should fail");
+
+        assert_eq!(
+            match &err {
+                DiskError::Io(io_err) => io_err.kind(),
+                other => panic!("expected io network error, got {other:?}"),
+            },
+            std::io::ErrorKind::ConnectionRefused
+        );
+        assert!(!remote_disk.is_online().await, "network-like errors should mark remote disk faulty");
+        assert!(
+            !GLOBAL_CONN_MAP.read().await.contains_key(&addr),
+            "network-like errors should evict cached connection"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_execute_with_timeout_keeps_remote_disk_online_for_business_error() {
+        let addr = "http://127.0.0.1:59994".to_string();
+        let url = url::Url::parse(&format!("{addr}/data")).unwrap();
+        let endpoint = Endpoint {
+            url,
+            is_local: false,
+            pool_idx: 0,
+            set_idx: 0,
+            disk_idx: 0,
+        };
+
+        let remote_disk = RemoteDisk::new(
+            &endpoint,
+            &DiskOption {
+                cleanup: false,
+                health_check: false,
+            },
+        )
+        .await
+        .unwrap();
+
+        let channel = TonicEndpoint::from_shared(addr.clone()).unwrap().connect_lazy();
+        GLOBAL_CONN_MAP.write().await.insert(addr.clone(), channel);
+
+        let err = remote_disk
+            .execute_with_timeout(|| async { Err::<(), Error>(DiskError::FileNotFound) }, Duration::from_secs(1))
+            .await
+            .expect_err("business error should still fail the operation");
+
+        assert_eq!(err, DiskError::FileNotFound);
+        assert!(remote_disk.is_online().await, "business errors should not mark remote disk faulty");
+        assert!(
+            GLOBAL_CONN_MAP.read().await.contains_key(&addr),
+            "business errors should not evict cached connection"
+        );
+    }
+
     #[test]
     fn test_remote_disk_sync_properties() {
         let url = url::Url::parse("https://secure-remote:9000/data").unwrap();