diff --git a/config/base/raft.toml b/config/base/raft.toml index 180fff73..e47abad7 100644 --- a/config/base/raft.toml +++ b/config/base/raft.toml @@ -40,10 +40,8 @@ allow_client_override = true state_machine_sync_timeout_ms = 10 [raft.state_machine.lease] -# Enable TTL (time-to-live) key expiration feature. -# When disabled (default), no lease components are initialized — zero overhead. -# When enabled, a background worker periodically removes expired keys. -enabled = false +# TTL key expiration — always active, no enable flag needed. +# A background worker periodically removes expired keys. # How often the background cleanup worker runs (milliseconds). # Controls MEMORY EFFICIENCY only — expired keys are always rejected at read time diff --git a/d-engine-core/src/config/lease.rs b/d-engine-core/src/config/lease.rs index 834a2c25..67920462 100644 --- a/d-engine-core/src/config/lease.rs +++ b/d-engine-core/src/config/lease.rs @@ -5,18 +5,16 @@ //! //! # Design Philosophy //! -//! - **Simple**: Single `enabled` flag - no strategy choices +//! - **Always On**: TTL is part of the public API — no feature flag needed //! - **Best Practice**: Fixed background cleanup (industry standard) -//! - **Zero Overhead**: When disabled, no lease components are initialized -//! - **Graceful Degradation**: TTL requests are rejected when disabled +//! - **Low Overhead**: When no TTL keys exist, cleanup cycle is a single atomic load (~10ns) //! //! # Usage //! //! ```toml -//! # Enable TTL feature (default: disabled) +//! # Optional: tune cleanup behaviour (TTL is always active) //! [raft.state_machine.lease] -//! enabled = true -//! cleanup_interval_ms = 1000 # Optional, default 1 second +//! cleanup_interval_ms = 1000 # default 1 second //! ``` use config::ConfigError; @@ -26,52 +24,27 @@ use serde::Serialize; use crate::errors::Error; use crate::errors::Result; -/// Lease configuration for TTL-based key expiration +/// Lease configuration for TTL-based key expiration. /// -/// When `enabled = true`: -/// - Server initializes lease manager at startup -/// - Background worker periodically cleans expired keys -/// - Client TTL requests (ttl_secs > 0) are accepted -/// -/// When `enabled = false` (default): -/// - No lease components initialized (zero overhead) -/// - Client TTL requests (ttl_secs > 0) are rejected with error -/// - All keys are permanent (ttl_secs = 0 is always allowed) +/// TTL is always active — no enable flag. Background cleanup runs automatically. /// /// # Performance /// /// Background cleanup overhead: ~0.001% CPU /// - Wakes up every `cleanup_interval_ms` (default 1000ms) -/// - Scans expired keys (limited by `max_cleanup_duration_ms`) -/// - Deletes expired entries from storage +/// - No-op when no TTL keys exist (~10ns atomic load) +/// - Deletes expired entries when keys are present /// /// # Examples /// /// ```rust /// use d_engine_core::config::LeaseConfig; /// -/// // Default: disabled /// let config = LeaseConfig::default(); -/// assert!(!config.enabled); -/// -/// // Enable with defaults -/// let config = LeaseConfig { -/// enabled: true, -/// ..Default::default() -/// }; /// assert_eq!(config.cleanup_interval_ms, 1000); /// ``` #[derive(Serialize, Deserialize, Clone, Debug)] pub struct LeaseConfig { - /// Enable TTL feature - /// - /// - `true`: Initialize lease manager + start background cleanup worker - /// - `false`: No TTL support, reject requests with ttl_secs > 0 - /// - /// Default: false (zero overhead when TTL not needed) - #[serde(default)] - pub enabled: bool, - /// How often the background worker wakes up to scan and delete expired keys (milliseconds). /// /// This setting controls MEMORY EFFICIENCY only, not TTL correctness. @@ -90,7 +63,7 @@ pub struct LeaseConfig { /// - 100-500ms: High-throughput short-TTL workloads (e.g., rate limiting, sessions) /// - 5000-60000ms: Low-frequency TTL usage where memory pressure is not a concern /// - /// Only used when `enabled = true` + /// Always active — no enable flag needed. #[serde(default = "default_cleanup_interval_ms")] pub cleanup_interval_ms: u64, @@ -107,7 +80,7 @@ pub struct LeaseConfig { /// - 5-10ms: Aggressive cleanup when backlog exists /// - >10ms: Not recommended (can impact foreground operations) /// - /// Only used when `enabled = true` + /// Always active — no enable flag needed. #[serde(default = "default_max_cleanup_duration_ms")] pub max_cleanup_duration_ms: u64, } @@ -123,7 +96,6 @@ fn default_max_cleanup_duration_ms() -> u64 { impl Default for LeaseConfig { fn default() -> Self { Self { - enabled: false, // Default: TTL disabled (zero overhead) cleanup_interval_ms: default_cleanup_interval_ms(), max_cleanup_duration_ms: default_max_cleanup_duration_ms(), } @@ -137,11 +109,6 @@ impl LeaseConfig { /// - `cleanup_interval_ms` is out of range (100-60000) /// - `max_cleanup_duration_ms` is out of range (1-100) pub fn validate(&self) -> Result<()> { - // Skip validation if disabled - if !self.enabled { - return Ok(()); - } - // Validate cleanup_interval_ms // Range: 100ms to 60s // - Lower bound (100ms): Prevents excessive wakeups (CPU waste) diff --git a/d-engine-core/src/config/lease_test.rs b/d-engine-core/src/config/lease_test.rs index 1edf7ff1..aca1fd79 100644 --- a/d-engine-core/src/config/lease_test.rs +++ b/d-engine-core/src/config/lease_test.rs @@ -3,91 +3,123 @@ use super::lease::LeaseConfig; #[test] fn test_default_config() { let config = LeaseConfig::default(); - assert!(!config.enabled); // Default: disabled assert_eq!(config.cleanup_interval_ms, 1000); assert_eq!(config.max_cleanup_duration_ms, 1); assert!(config.validate().is_ok()); } #[test] -fn test_enabled_config() { +fn test_explicit_config() { let config = LeaseConfig { - enabled: true, cleanup_interval_ms: 5000, max_cleanup_duration_ms: 5, }; - assert!(config.enabled); assert_eq!(config.cleanup_interval_ms, 5000); assert_eq!(config.max_cleanup_duration_ms, 5); assert!(config.validate().is_ok()); } -#[test] -fn test_disabled_config_skips_validation() { - let config = LeaseConfig { - enabled: false, - cleanup_interval_ms: 50, // Invalid (too small), but ignored when disabled - max_cleanup_duration_ms: 0, // Invalid, but ignored when disabled - }; - assert!(config.validate().is_ok()); -} - #[test] fn test_validation_cleanup_interval_ms() { - let mut config = LeaseConfig { - enabled: true, - ..Default::default() - }; + let base = LeaseConfig::default(); // Too small - config.cleanup_interval_ms = 99; - assert!(config.validate().is_err()); - + assert!( + LeaseConfig { + cleanup_interval_ms: 99, + ..base + } + .validate() + .is_err() + ); // Too large - config.cleanup_interval_ms = 60_001; - assert!(config.validate().is_err()); - + assert!( + LeaseConfig { + cleanup_interval_ms: 60_001, + ..base + } + .validate() + .is_err() + ); // Valid range - config.cleanup_interval_ms = 100; - assert!(config.validate().is_ok()); - - config.cleanup_interval_ms = 1000; - assert!(config.validate().is_ok()); - - config.cleanup_interval_ms = 60_000; - assert!(config.validate().is_ok()); + assert!( + LeaseConfig { + cleanup_interval_ms: 100, + ..base + } + .validate() + .is_ok() + ); + assert!( + LeaseConfig { + cleanup_interval_ms: 1000, + ..base + } + .validate() + .is_ok() + ); + assert!( + LeaseConfig { + cleanup_interval_ms: 60_000, + ..base + } + .validate() + .is_ok() + ); } #[test] fn test_validation_max_cleanup_duration() { - let mut config = LeaseConfig { - enabled: true, - ..Default::default() - }; + let base = LeaseConfig::default(); // Too small - config.max_cleanup_duration_ms = 0; - assert!(config.validate().is_err()); - + assert!( + LeaseConfig { + max_cleanup_duration_ms: 0, + ..base + } + .validate() + .is_err() + ); // Too large - config.max_cleanup_duration_ms = 101; - assert!(config.validate().is_err()); - + assert!( + LeaseConfig { + max_cleanup_duration_ms: 101, + ..base + } + .validate() + .is_err() + ); // Valid range - config.max_cleanup_duration_ms = 1; - assert!(config.validate().is_ok()); - - config.max_cleanup_duration_ms = 50; - assert!(config.validate().is_ok()); - - config.max_cleanup_duration_ms = 100; - assert!(config.validate().is_ok()); + assert!( + LeaseConfig { + max_cleanup_duration_ms: 1, + ..base + } + .validate() + .is_ok() + ); + assert!( + LeaseConfig { + max_cleanup_duration_ms: 50, + ..base + } + .validate() + .is_ok() + ); + assert!( + LeaseConfig { + max_cleanup_duration_ms: 100, + ..base + } + .validate() + .is_ok() + ); } #[test] fn test_typical_production_config() { let config = LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -97,7 +129,6 @@ fn test_typical_production_config() { #[test] fn test_aggressive_cleanup_config() { let config = LeaseConfig { - enabled: true, cleanup_interval_ms: 100, max_cleanup_duration_ms: 10, }; @@ -107,7 +138,6 @@ fn test_aggressive_cleanup_config() { #[test] fn test_relaxed_cleanup_config() { let config = LeaseConfig { - enabled: true, cleanup_interval_ms: 60_000, max_cleanup_duration_ms: 5, }; diff --git a/d-engine-core/src/errors.rs b/d-engine-core/src/errors.rs index 1da23407..14ab48b4 100644 --- a/d-engine-core/src/errors.rs +++ b/d-engine-core/src/errors.rs @@ -247,14 +247,6 @@ pub enum StorageError { #[error(transparent)] IdAllocation(#[from] IdAllocationError), - /// Feature not enabled in configuration - /// - /// Returned when client requests a feature (e.g., TTL) that is not - /// enabled in the server configuration. This prevents silent failures - /// and ensures explicit feature activation. - #[error("Feature not enabled: {0}")] - FeatureNotEnabled(String), - /// State machine not serving requests /// /// Returned when read operations are attempted during critical operations diff --git a/d-engine-server/benches/lease_performance.rs b/d-engine-server/benches/lease_performance.rs index c54ee9ef..fac06eac 100644 --- a/d-engine-server/benches/lease_performance.rs +++ b/d-engine-server/benches/lease_performance.rs @@ -27,7 +27,6 @@ use tempfile::TempDir; async fn create_sm_background() -> (FileStateMachine, TempDir) { let temp_dir = TempDir::new().expect("Failed to create temp dir"); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; diff --git a/d-engine-server/benches/state_machine.rs b/d-engine-server/benches/state_machine.rs index d86a08f7..f5836212 100644 --- a/d-engine-server/benches/state_machine.rs +++ b/d-engine-server/benches/state_machine.rs @@ -37,7 +37,6 @@ async fn create_test_state_machine() -> (FileStateMachine, TempDir) { // Enable TTL for benchmarks that need it let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; diff --git a/d-engine-server/benches/ttl.rs b/d-engine-server/benches/ttl.rs index 3fa76823..c01f9130 100644 --- a/d-engine-server/benches/ttl.rs +++ b/d-engine-server/benches/ttl.rs @@ -28,7 +28,6 @@ async fn create_test_state_machine() -> (FileStateMachine, TempDir) { let temp_dir = TempDir::new().expect("Failed to create temp dir"); // For TTL benchmarks, we need lease enabled let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -88,7 +87,6 @@ fn bench_piggyback_cleanup(c: &mut Criterion) { // Test with different numbers of expired keys for expired_count in [10, 50, 100, 500].iter() { let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -126,7 +124,6 @@ fn bench_ttl_registration(c: &mut Criterion) { use d_engine_server::storage::DefaultLease; let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; diff --git a/d-engine-server/src/api/embedded.rs b/d-engine-server/src/api/embedded.rs index ee3cab6b..2599d439 100644 --- a/d-engine-server/src/api/embedded.rs +++ b/d-engine-server/src/api/embedded.rs @@ -227,11 +227,10 @@ impl EmbeddedEngine { (storage, sm) }; - let lease_cfg = &config.raft.state_machine.lease; - if lease_cfg.enabled { - let lease = Arc::new(crate::storage::DefaultLease::new(lease_cfg.clone())); - sm.set_lease(lease); - } + let lease = Arc::new(crate::storage::DefaultLease::new( + config.raft.state_machine.lease.clone(), + )); + sm.set_lease(lease); Self::start_node(config, Arc::new(storage), Arc::new(sm)).await } @@ -277,12 +276,10 @@ impl EmbeddedEngine { (storage, sm) }; - // Inject lease if enabled - let lease_cfg = &config.raft.state_machine.lease; - if lease_cfg.enabled { - let lease = Arc::new(crate::storage::DefaultLease::new(lease_cfg.clone())); - sm.set_lease(lease); - } + let lease = Arc::new(crate::storage::DefaultLease::new( + config.raft.state_machine.lease.clone(), + )); + sm.set_lease(lease); Self::start_custom(Arc::new(storage), Arc::new(sm), Some(config_path)).await } diff --git a/d-engine-server/src/api/standalone.rs b/d-engine-server/src/api/standalone.rs index dfe91a91..f01958f2 100644 --- a/d-engine-server/src/api/standalone.rs +++ b/d-engine-server/src/api/standalone.rs @@ -68,11 +68,10 @@ impl StandaloneEngine { (storage, sm) }; - let lease_cfg = &config.raft.state_machine.lease; - if lease_cfg.enabled { - let lease = Arc::new(crate::storage::DefaultLease::new(lease_cfg.clone())); - sm.set_lease(lease); - } + let lease = Arc::new(crate::storage::DefaultLease::new( + config.raft.state_machine.lease.clone(), + )); + sm.set_lease(lease); Self::start_node(config, Arc::new(storage), Arc::new(sm), shutdown_rx).await } @@ -123,12 +122,10 @@ impl StandaloneEngine { (storage, sm) }; - // Inject lease if enabled - let lease_cfg = &config.raft.state_machine.lease; - if lease_cfg.enabled { - let lease = Arc::new(crate::storage::DefaultLease::new(lease_cfg.clone())); - sm.set_lease(lease); - } + let lease = Arc::new(crate::storage::DefaultLease::new( + config.raft.state_machine.lease.clone(), + )); + sm.set_lease(lease); Self::start_node(config, Arc::new(storage), Arc::new(sm), shutdown_rx).await } diff --git a/d-engine-server/src/node/builder.rs b/d-engine-server/src/node/builder.rs index 5097bf5b..2a35ce8e 100644 --- a/d-engine-server/src/node/builder.rs +++ b/d-engine-server/src/node/builder.rs @@ -281,22 +281,16 @@ where // Start state machine: flip flags and load persisted data state_machine.start().await?; - // Spawn lease background cleanup task (if TTL feature is enabled) - // Framework-level feature: completely transparent to developers - let lease_cleanup_handle = if node_config.raft.state_machine.lease.enabled { - info!( - "Starting lease background cleanup worker (interval: {}ms)", - node_config.raft.state_machine.lease.cleanup_interval_ms - ); - Some(Self::spawn_background_cleanup_worker( - Arc::clone(&state_machine), - node_config.raft.state_machine.lease.cleanup_interval_ms, - self.shutdown_signal.clone(), - )) - } else { - debug!("Lease feature disabled: no background cleanup worker"); - None - }; + // Spawn lease background cleanup task (TTL is always active) + info!( + "Starting lease background cleanup worker (interval: {}ms)", + node_config.raft.state_machine.lease.cleanup_interval_ms + ); + let lease_cleanup_handle = Some(Self::spawn_background_cleanup_worker( + Arc::clone(&state_machine), + node_config.raft.state_machine.lease.cleanup_interval_ms, + self.shutdown_signal.clone(), + )); // Handle storage engine initialization let storage_engine = self.storage_engine.take().ok_or_else(|| { diff --git a/d-engine-server/src/storage/adaptors/file/file_state_machine.rs b/d-engine-server/src/storage/adaptors/file/file_state_machine.rs index f2fc5bed..9b4952c3 100644 --- a/d-engine-server/src/storage/adaptors/file/file_state_machine.rs +++ b/d-engine-server/src/storage/adaptors/file/file_state_machine.rs @@ -246,13 +246,6 @@ pub struct FileStateMachine { // Injected by NodeBuilder after construction lease: Option>, - /// Whether lease manager is enabled (immutable after init) - /// Set to true when lease is injected, never changes after that - /// - /// Invariant: lease_enabled == true ⟹ lease.is_some() - /// Performance: Allows safe unwrap_unchecked in hot paths - lease_enabled: bool, - // Raft state with disk persistence last_applied_index: AtomicU64, last_applied_term: AtomicU64, @@ -289,8 +282,7 @@ impl FileStateMachine { let machine = Self { data: RwLock::new(HashMap::new()), - lease: None, // Will be injected by NodeBuilder - lease_enabled: false, // Default: no lease until set + lease: None, last_applied_index: AtomicU64::new(0), last_applied_term: AtomicU64::new(0), last_snapshot_metadata: RwLock::new(None), @@ -315,8 +307,6 @@ impl FileStateMachine { &mut self, lease: Arc, ) { - // Mark lease as enabled (immutable after this point) - self.lease_enabled = true; self.lease = Some(lease); } @@ -1201,16 +1191,10 @@ impl StateMachine for FileStateMachine { } => { data.insert(key.clone(), (value.clone(), entry.term)); if let Some(ttl) = ttl_secs { - if !self.lease_enabled { - return Err(StorageError::FeatureNotEnabled( - "TTL feature is not enabled on this server. \ - Enable it in config: [raft.state_machine.lease] enabled = true" - .into(), - ) - .into()); - } - // Safety: lease_enabled invariant ensures lease.is_some() - let lease = unsafe { self.lease.as_ref().unwrap_unchecked() }; + let lease = self + .lease + .as_ref() + .expect("lease always initialized by NodeBuilder"); lease.register(key.clone(), *ttl); } results.push(ApplyResult::success(entry.index)); @@ -1538,13 +1522,22 @@ impl StateMachine for FileStateMachine { } async fn lease_background_cleanup(&self) -> Result, Error> { - // Fast path: no lease configured let Some(ref lease) = self.lease else { return Ok(vec![]); }; - // Get all expired keys + // Fast path: no TTL keys ever registered — single atomic load (~10ns) + if !lease.has_lease_keys() { + return Ok(vec![]); + } + let now = SystemTime::now(); + + // Fast path: sample first 10 entries — if none expired, skip full scan (~30ns) + if !lease.may_have_expired_keys(now) { + return Ok(vec![]); + } + let expired_keys = lease.get_expired_keys(now); if expired_keys.is_empty() { diff --git a/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_state_machine.rs b/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_state_machine.rs index 53ff567d..72975850 100644 --- a/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_state_machine.rs +++ b/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_state_machine.rs @@ -86,13 +86,6 @@ pub struct RocksDBStateMachine { // DefaultLease is thread-safe internally (uses DashMap + Mutex) // Injected by NodeBuilder after construction lease: Option>, - - /// Whether lease manager is enabled (immutable after init) - /// Set to true when lease is injected, never changes after that - /// - /// Invariant: lease_enabled == true ⟹ lease.is_some() - /// Performance: Allows safe unwrap_unchecked in hot paths - lease_enabled: bool, } /// Returns the lexicographic successor of `prefix` for use as an iterator upper bound. @@ -143,7 +136,6 @@ impl RocksDBStateMachine { last_applied_term: AtomicU64::new(last_applied_term), last_snapshot_metadata: RwLock::new(last_snapshot_metadata), lease: None, - lease_enabled: false, }) } @@ -162,7 +154,6 @@ impl RocksDBStateMachine { last_applied_term: AtomicU64::new(last_applied_term), last_snapshot_metadata: RwLock::new(last_snapshot_metadata), lease: None, - lease_enabled: false, }) } @@ -175,8 +166,6 @@ impl RocksDBStateMachine { &mut self, lease: Arc, ) { - // Mark lease as enabled (immutable after this point) - self.lease_enabled = true; self.lease = Some(lease); } @@ -755,16 +744,8 @@ impl StateMachine for RocksDBStateMachine { batch.put_cf(&cf, key, value); if let Some(ttl) = ttl_secs { - if !self.lease_enabled { - return Err(StorageError::FeatureNotEnabled( - "TTL feature is not enabled on this server. \ - Enable it in config: [raft.state_machine.lease] enabled = true" - .into(), - ) - .into()); - } - // Safety: lease_enabled invariant ensures lease.is_some() - let lease = unsafe { self.lease.as_ref().unwrap_unchecked() }; + let lease = + self.lease.as_ref().expect("lease always initialized by NodeBuilder"); lease.register(key.clone(), *ttl); } @@ -1040,13 +1021,22 @@ impl StateMachine for RocksDBStateMachine { } async fn lease_background_cleanup(&self) -> Result, Error> { - // Fast path: no lease configured let Some(ref lease) = self.lease else { return Ok(vec![]); }; - // Get all expired keys + // Fast path: no TTL keys ever registered — single atomic load (~10ns) + if !lease.has_lease_keys() { + return Ok(vec![]); + } + let now = SystemTime::now(); + + // Fast path: sample first 10 entries — if none expired, skip full scan (~30ns) + if !lease.may_have_expired_keys(now) { + return Ok(vec![]); + } + let expired_keys = lease.get_expired_keys(now); if expired_keys.is_empty() { diff --git a/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_unified_engine_test.rs b/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_unified_engine_test.rs index 081e5abe..d38d60d6 100644 --- a/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_unified_engine_test.rs +++ b/d-engine-server/src/storage/adaptors/rocksdb/rocksdb_unified_engine_test.rs @@ -171,7 +171,6 @@ fn test_concurrent_open_same_path_fails() { fn make_lease() -> Arc { Arc::new(DefaultLease::new(LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 10, })) diff --git a/d-engine-server/src/storage/lease_integration_test.rs b/d-engine-server/src/storage/lease_integration_test.rs index 4ca832af..bb5e1cd8 100644 --- a/d-engine-server/src/storage/lease_integration_test.rs +++ b/d-engine-server/src/storage/lease_integration_test.rs @@ -145,7 +145,6 @@ mod file_state_machine_tests { async fn test_ttl_expiration_after_apply() { let temp_dir = TempDir::new().unwrap(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -175,7 +174,6 @@ mod file_state_machine_tests { async fn test_ttl_snapshot_persistence() { let temp_dir = TempDir::new().unwrap(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -224,7 +222,6 @@ mod file_state_machine_tests { let temp_dir = TempDir::new().unwrap(); let state_machine_path = temp_dir.path().to_path_buf(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -297,7 +294,6 @@ mod file_state_machine_tests { async fn test_ttl_update_cancels_previous() { let temp_dir = TempDir::new().unwrap(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -328,7 +324,6 @@ mod file_state_machine_tests { let temp_dir = TempDir::new().unwrap(); let state_machine_path = temp_dir.path().to_path_buf(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -589,7 +584,6 @@ mod file_state_machine_tests { async fn test_background_strategy_manual_cleanup() { let temp_dir = TempDir::new().unwrap(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 50, }; @@ -642,7 +636,6 @@ mod file_state_machine_tests { async fn test_background_cleanup_respects_max_duration() { let temp_dir = TempDir::new().unwrap(); let lease_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, // Very short limit }; @@ -678,6 +671,92 @@ mod file_state_machine_tests { duration ); } + + /// Fast path: cleanup is a no-op when no TTL keys have ever been registered. + /// + /// `has_lease_keys()` returns false → cleanup must return empty without + /// scanning storage. Regular (non-TTL) keys must be untouched. + #[tokio::test] + async fn test_background_cleanup_fast_path_no_ttl_keys_ever_registered() { + let temp_dir = TempDir::new().unwrap(); + let sm = create_file_state_machine_with_lease( + temp_dir.path().to_path_buf(), + d_engine_core::config::LeaseConfig::default(), + ) + .await; + + // Insert regular keys without TTL + for i in 0..5u64 { + let key = format!("plain_key_{i}"); + let entry = ApplyEntry { + index: i + 1, + term: 1, + command: Command::Insert { + key: Bytes::copy_from_slice(key.as_bytes()), + value: Bytes::from("value"), + ttl_secs: None, + }, + }; + sm.apply_chunk(&[entry]).await.unwrap(); + } + + // Cleanup must be a no-op: no TTL keys, nothing to scan or delete + let cleaned = sm.lease_background_cleanup().await.unwrap(); + assert!( + cleaned.is_empty(), + "cleanup must return empty when no TTL keys registered, got {:?}", + cleaned + ); + + // All regular keys must survive cleanup + for i in 0..5u64 { + let key = format!("plain_key_{i}"); + assert_eq!( + sm.get(key.as_bytes()).unwrap(), + Some(Bytes::from("value")), + "plain_key_{i} must not be touched by cleanup" + ); + } + } + + /// Fast path: cleanup is a no-op when TTL keys exist but none are expired. + /// + /// `has_lease_keys()` is true but `may_have_expired_keys()` is false → + /// cleanup must return empty and leave all keys intact. + #[tokio::test] + async fn test_background_cleanup_fast_path_unexpired_keys_preserved() { + let temp_dir = TempDir::new().unwrap(); + let sm = create_file_state_machine_with_lease( + temp_dir.path().to_path_buf(), + d_engine_core::config::LeaseConfig::default(), + ) + .await; + + // Insert keys with a very long TTL (3600s, will not expire during test) + for i in 0..5u64 { + let key = format!("ttl_key_{i}"); + let entry = create_insert_entry(i + 1, 1, key.as_bytes(), b"value", 3600); + sm.apply_chunk(&[entry]).await.unwrap(); + } + + // Cleanup must skip all keys: none are expired + let cleaned = sm.lease_background_cleanup().await.unwrap(); + assert!( + cleaned.is_empty(), + "cleanup must return empty when no keys are expired, got {:?}", + cleaned + ); + + // All TTL keys must still be readable + for i in 0..5u64 { + let key = format!("ttl_key_{i}"); + assert_eq!( + sm.get(key.as_bytes()).unwrap(), + Some(Bytes::from("value")), + "ttl_key_{i} must not be deleted by cleanup before expiry" + ); + } + } } #[cfg(all(test, feature = "rocksdb"))] @@ -742,7 +821,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_ttl_expiration_after_apply() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -774,7 +852,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_ttl_snapshot_persistence() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -828,7 +905,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_ttl_update_cancels_previous() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -860,7 +936,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_ttl_delete_unregisters() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -890,7 +965,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_multiple_keys_with_different_ttls() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -939,7 +1013,6 @@ mod rocksdb_state_machine_tests { let temp_dir = TempDir::new().unwrap(); let state_machine_path = temp_dir.path().join("rocksdb"); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -1013,7 +1086,6 @@ mod rocksdb_state_machine_tests { async fn test_rocksdb_reset_clears_ttl_manager() { let temp_dir = TempDir::new().unwrap(); let ttl_config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -1035,11 +1107,96 @@ mod rocksdb_state_machine_tests { assert_eq!(sm.len(), 0); } + /// Fast path: cleanup is a no-op when no TTL keys have ever been registered. + /// + /// `has_lease_keys()` returns false → cleanup must return empty without + /// scanning storage. Regular (non-TTL) keys must be untouched. + #[tokio::test] + async fn test_rocksdb_background_cleanup_fast_path_no_ttl_keys_ever_registered() { + let temp_dir = TempDir::new().unwrap(); + let (_storage, sm) = create_rocksdb_state_machine_with_lease( + temp_dir.path().join("rocksdb"), + d_engine_core::config::LeaseConfig::default(), + ) + .await; + + // Insert regular keys without TTL + for i in 0..5u64 { + let key = format!("plain_key_{i}"); + let entry = ApplyEntry { + index: i + 1, + term: 1, + command: Command::Insert { + key: Bytes::copy_from_slice(key.as_bytes()), + value: Bytes::from("value"), + ttl_secs: None, + }, + }; + sm.apply_chunk(&[entry]).await.unwrap(); + } + + // Cleanup must be a no-op: no TTL keys, nothing to scan or delete + let cleaned = sm.lease_background_cleanup().await.unwrap(); + assert!( + cleaned.is_empty(), + "cleanup must return empty when no TTL keys registered, got {:?}", + cleaned + ); + + // All regular keys must survive cleanup + for i in 0..5u64 { + let key = format!("plain_key_{i}"); + assert_eq!( + sm.get(key.as_bytes()).unwrap(), + Some(Bytes::from("value")), + "plain_key_{i} must not be touched by cleanup" + ); + } + } + + /// Fast path: cleanup is a no-op when TTL keys exist but none are expired. + /// + /// `has_lease_keys()` is true but `may_have_expired_keys()` is false → + /// cleanup must return empty and leave all keys intact. + #[tokio::test] + async fn test_rocksdb_background_cleanup_fast_path_unexpired_keys_preserved() { + let temp_dir = TempDir::new().unwrap(); + let (_storage, sm) = create_rocksdb_state_machine_with_lease( + temp_dir.path().join("rocksdb"), + d_engine_core::config::LeaseConfig::default(), + ) + .await; + + // Insert keys with a very long TTL (3600s, will not expire during test) + for i in 0..5u64 { + let key = format!("ttl_key_{i}"); + let entry = create_insert_entry(i + 1, 1, key.as_bytes(), b"value", 3600); + sm.apply_chunk(&[entry]).await.unwrap(); + } + + // Cleanup must skip all keys: none are expired + let cleaned = sm.lease_background_cleanup().await.unwrap(); + assert!( + cleaned.is_empty(), + "cleanup must return empty when no keys are expired, got {:?}", + cleaned + ); + + // All TTL keys must still be readable + for i in 0..5u64 { + let key = format!("ttl_key_{i}"); + assert_eq!( + sm.get(key.as_bytes()).unwrap(), + Some(Bytes::from("value")), + "ttl_key_{i} must not be deleted by cleanup before expiry" + ); + } + } + // #[tokio::test] // async fn test_rocksdb_passive_deletion_on_get() { // let temp_dir = TempDir::new().unwrap(); // let ttl_config = d_engine_core::config::LeaseConfig { - // enabled: true, // cleanup_interval_ms: 1000, // max_cleanup_duration_ms: 1, // }; diff --git a/d-engine-server/src/storage/lease_unit_test.rs b/d-engine-server/src/storage/lease_unit_test.rs index 53100d03..e83469b2 100644 --- a/d-engine-server/src/storage/lease_unit_test.rs +++ b/d-engine-server/src/storage/lease_unit_test.rs @@ -18,7 +18,6 @@ use crate::storage::lease::DefaultLease; fn default_config() -> d_engine_core::config::LeaseConfig { d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, } @@ -303,7 +302,6 @@ fn test_reload_invalid_data() { #[test] fn test_reload_clears_apply_counter() { let config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 1000, max_cleanup_duration_ms: 1, }; @@ -350,14 +348,12 @@ fn test_on_apply_piggyback_removed() { } #[test] -fn test_enabled_config() { +fn test_custom_cleanup_interval() { let config = d_engine_core::config::LeaseConfig { - enabled: true, cleanup_interval_ms: 5000, max_cleanup_duration_ms: 1, }; let lease = DefaultLease::new(config); - assert!(lease.config().enabled); assert_eq!(lease.config().cleanup_interval_ms, 5000); } diff --git a/d-engine-server/tests/consistent_reads/lease_read_embedded.rs b/d-engine-server/tests/consistent_reads/lease_read_embedded.rs index 39b36625..1c70afa0 100644 --- a/d-engine-server/tests/consistent_reads/lease_read_embedded.rs +++ b/d-engine-server/tests/consistent_reads/lease_read_embedded.rs @@ -31,9 +31,6 @@ listen_address = "127.0.0.1:{}" db_root_dir = "{}" single_node = true -[raft.state_machine.lease] -enabled = true - [raft.read_consistency] state_machine_sync_timeout_ms = 2000 "#, diff --git a/d-engine-server/tests/drain_batching/select_fairness_embedded.rs b/d-engine-server/tests/drain_batching/select_fairness_embedded.rs index 2bac296f..e2b9e295 100644 --- a/d-engine-server/tests/drain_batching/select_fairness_embedded.rs +++ b/d-engine-server/tests/drain_batching/select_fairness_embedded.rs @@ -37,8 +37,6 @@ single_node = true [raft.batching] max_batch_size = 100 -[raft.state_machine.lease] -enabled = true "#, port, db_path.display() diff --git a/d-engine-server/tests/embedded_client/embedded_client_operations.rs b/d-engine-server/tests/embedded_client/embedded_client_operations.rs index 813d71c8..59a82b1f 100644 --- a/d-engine-server/tests/embedded_client/embedded_client_operations.rs +++ b/d-engine-server/tests/embedded_client/embedded_client_operations.rs @@ -14,6 +14,36 @@ use tempfile::TempDir; use crate::common::get_available_ports; +/// Helper to create a test EmbeddedEngine without any lease config section. +/// +/// Used by regression tests that must verify behaviour with default (zero-config) +/// settings — notably #398 where omitting [raft.state_machine.lease] previously +/// caused a fatal crash when put_with_ttl was called. +async fn create_test_engine_default_config(test_name: &str) -> (EmbeddedEngine, TempDir) { + let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let db_path = temp_dir.path().join(test_name); + let config_path = temp_dir.path().join("d-engine.toml"); + let mut port_guard = get_available_ports(1).await; + port_guard.release_listeners(); + let port = port_guard.as_slice()[0]; + let config_content = format!( + r#" +[cluster] +listen_address = "127.0.0.1:{}" +db_root_dir = "{}" +single_node = true +"#, + port, + db_path.display() + ); + std::fs::write(&config_path, config_content).expect("Failed to write config"); + let engine = EmbeddedEngine::start_with(config_path.to_str().unwrap()) + .await + .expect("Failed to start engine"); + engine.wait_ready(Duration::from_secs(5)).await.expect("Engine not ready"); + (engine, temp_dir) +} + /// Helper to create a test EmbeddedEngine async fn create_test_engine(test_name: &str) -> (EmbeddedEngine, TempDir) { let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); @@ -29,9 +59,6 @@ async fn create_test_engine(test_name: &str) -> (EmbeddedEngine, TempDir) { listen_address = "127.0.0.1:{}" db_root_dir = "{}" single_node = true - -[raft.state_machine.lease] -enabled = true "#, port, db_path.display() @@ -536,6 +563,49 @@ async fn test_put_with_ttl_readable_immediately() { ); } +/// Regression test for #398. +/// +/// put_with_ttl must succeed when no [raft.state_machine.lease] section is present +/// in the config (i.e. default/zero-config). Previously this caused a fatal crash: +/// the state machine returned an error which the SM Worker mapped to +/// RoleEvent::FatalError, shutting down the entire node. +/// +/// Verification steps: +/// 1. Engine starts with default config (no lease section) +/// 2. put_with_ttl returns Ok(()) — not a crash, not an error +/// 3. The written value is readable — entry was applied correctly +/// 4. A subsequent put/get succeeds — node is still alive after the TTL write +#[tokio::test] +async fn test_put_with_ttl_succeeds_with_default_config() { + let (engine, _temp_dir) = + create_test_engine_default_config("put_with_ttl_default_config").await; + let client = engine.client(); + + // Must not crash the node, must return Ok + client + .put_with_ttl(b"ttl-key", b"ttl-value", 30) + .await + .expect("put_with_ttl must succeed with default config (#398 regression)"); + + // Value must be readable immediately (entry was applied) + let value = client + .get_linearizable(b"ttl-key") + .await + .expect("get after put_with_ttl should succeed"); + assert_eq!(value, Some(Bytes::from("ttl-value"))); + + // Node must still be alive — a regular put/get must work after the TTL write + client + .put(b"probe", b"alive") + .await + .expect("node must still be alive after put_with_ttl (#398 regression)"); + let probe = client + .get_linearizable(b"probe") + .await + .expect("probe read should succeed after put_with_ttl"); + assert_eq!(probe, Some(Bytes::from("alive"))); +} + // ============================================================================= // get_with_consistency (LeaseRead) // ============================================================================= diff --git a/examples/three-nodes-standalone/config/n1.toml b/examples/three-nodes-standalone/config/n1.toml index dcb9630c..0d47e2f5 100644 --- a/examples/three-nodes-standalone/config/n1.toml +++ b/examples/three-nodes-standalone/config/n1.toml @@ -54,7 +54,6 @@ retained_log_entries = 3 # == TTL Lease Configuration == [raft.state_machine.lease] -enabled = true cleanup_interval_ms = 1000 max_cleanup_duration_ms = 1 diff --git a/examples/three-nodes-standalone/config/n2.toml b/examples/three-nodes-standalone/config/n2.toml index 75d6ddcf..83342a3d 100644 --- a/examples/three-nodes-standalone/config/n2.toml +++ b/examples/three-nodes-standalone/config/n2.toml @@ -53,7 +53,6 @@ retained_log_entries = 3 # == TTL Lease Configuration == [raft.state_machine.lease] -enabled = true cleanup_interval_ms = 1000 max_cleanup_duration_ms = 1 diff --git a/examples/three-nodes-standalone/config/n3.toml b/examples/three-nodes-standalone/config/n3.toml index 0de49b9d..a6f7c9ee 100644 --- a/examples/three-nodes-standalone/config/n3.toml +++ b/examples/three-nodes-standalone/config/n3.toml @@ -53,7 +53,6 @@ retained_log_entries = 3 # == TTL Lease Configuration == [raft.state_machine.lease] -enabled = true cleanup_interval_ms = 1000 max_cleanup_duration_ms = 1