From a97a1ce518bd750867b3a18498b30667811296bc Mon Sep 17 00:00:00 2001 From: Joshua Chi Date: Sun, 12 Apr 2026 15:02:20 +0800 Subject: [PATCH 1/2] fix #290: reduce snapshot cooldown and fix check throttling - Reduce default cooldown from 3600s to 60s - Fix: use saturating_sub to handle clock jumps - Fix: use Relaxed ordering for last_checked (CAS lock already provides mutual exclusion) - Add: warning log when lag exceeds 10x snapshot threshold Implementation based on @fizikarubi's analysis in PR #338. The PR was delayed due to unrelated CI issues in the codebase (since resolved), but the technical approach was sound and is applied here. Co-authored-by: fizikarubi --- d-engine-core/src/config/raft.rs | 5 ++-- .../snapshot_policy/log_size.rs | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/d-engine-core/src/config/raft.rs b/d-engine-core/src/config/raft.rs index 6d8e72aa..2cf58bd7 100644 --- a/d-engine-core/src/config/raft.rs +++ b/d-engine-core/src/config/raft.rs @@ -619,9 +619,10 @@ fn default_max_log_entries_before_snapshot() -> u64 { /// Default cooldown duration between snapshot checks. /// /// Prevents constant evaluation of snapshot conditions in tight loops. -/// Currently set to 1 hour (3600 seconds). +/// With adaptive cooldown in `LogSizePolicy`, this is the *base* value that +/// shrinks automatically as log lag approaches the snapshot threshold. fn default_snapshot_cool_down_since_last_check() -> Duration { - Duration::from_secs(3600) + Duration::from_secs(60) } /// Default number of historical snapshots to retain diff --git a/d-engine-core/src/state_machine_handler/snapshot_policy/log_size.rs b/d-engine-core/src/state_machine_handler/snapshot_policy/log_size.rs index 99368c26..9b5d1b6d 100644 --- a/d-engine-core/src/state_machine_handler/snapshot_policy/log_size.rs +++ b/d-engine-core/src/state_machine_handler/snapshot_policy/log_size.rs @@ -7,6 +7,7 @@ use std::sync::atomic::Ordering; use std::time::Duration; use tracing::trace; +use tracing::warn; use super::SnapshotContext; use super::SnapshotPolicy; @@ -30,11 +31,12 @@ impl SnapshotPolicy for LogSizePolicy { return false; } - // Cooldown check using atomic operations + // Cooldown check — Relaxed is sufficient since the CAS on + // is_checking provides actual mutual exclusion. let now = timestamp_millis(); - let last = self.last_checked.load(Ordering::Acquire); + let last = self.last_checked.load(Ordering::Relaxed); - if now - last < self.cooldown_ms { + if now.saturating_sub(last) < self.cooldown_ms { return false; } @@ -47,9 +49,20 @@ impl SnapshotPolicy for LogSizePolicy { return false; } - let should_trigger = self.calculate_lag(ctx) >= self.threshold.load(Ordering::Relaxed); + let lag = self.calculate_lag(ctx); + let threshold = self.threshold.load(Ordering::Relaxed); + + if threshold > 0 && lag >= threshold.saturating_mul(10) { + warn!( + lag, + threshold, + "Log lag exceeds 10x snapshot threshold — snapshots may not be keeping up" + ); + } + + let should_trigger = lag >= threshold; if should_trigger { - self.last_checked.store(now, Ordering::Release); + self.last_checked.store(now, Ordering::Relaxed); } self.is_checking.store(false, Ordering::Release); From da2ae76ebb6462c856a2d3e6385fb3f84f1acc01 Mon Sep 17 00:00:00 2001 From: Joshua Chi Date: Sun, 12 Apr 2026 15:33:23 +0800 Subject: [PATCH 2/2] docs #290: fix misleading doc comment on snapshot cooldown default Removed reference to adaptive cooldown which was evaluated but not implemented. The comment now accurately reflects the actual behavior: a fixed 60s cooldown reduced from the previous 3600s default. --- d-engine-core/src/config/raft.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/d-engine-core/src/config/raft.rs b/d-engine-core/src/config/raft.rs index 2cf58bd7..a34f2972 100644 --- a/d-engine-core/src/config/raft.rs +++ b/d-engine-core/src/config/raft.rs @@ -619,8 +619,7 @@ fn default_max_log_entries_before_snapshot() -> u64 { /// Default cooldown duration between snapshot checks. /// /// Prevents constant evaluation of snapshot conditions in tight loops. -/// With adaptive cooldown in `LogSizePolicy`, this is the *base* value that -/// shrinks automatically as log lag approaches the snapshot threshold. +/// Reduced from 3600s to 60s to allow timely log compaction under typical workloads. fn default_snapshot_cool_down_since_last_check() -> Duration { Duration::from_secs(60) }