Skip to content

Commit 09f9cd2

Browse files
committed
fix(ttl_map): initialize time to 1 for correct wrapping sub
Fixes #221. Previously, we encountered a bug at the initial gc tick on ttl_map where `time` (denoting the number of ticks since map initialization) starts at 0, but the logic to compute the time to remove the inserted value is ``` free_time = (time - 1) // (number of ticks to ttl) ``` In other words, we maintain a circular buffer of inserts, with a single slot per tick, and we will delete the item once `buffer.len()` ticks have elapsed (ttl = tick * buffer.len()). This is all fine, except we implement the logic as: ``` free_time = time.wrapping_sub(1) / buffer.len() ``` which is computing for `u64` `time`: ``` free_time = ((time - 1) mod 2^64) mod buffer.len() ``` when we really just want `free_time = (time - 1) mod buffer.len()`. Luckily this equality holds as long as ` 0 < time < 2^64`, as for those times `time - 1 mod 2&64 = time`. This commit changes our behavior to initialize `time` at 1 instead of 0. We don't need to worry about the overflow case because even if the tick duration was a nanosecond, it would take ~584 years to overflow 64 bits at which point we would surely have other problems besides the momentarily incorrect ttl. For the underflow case, this should primarily help with unit tests above anything else, as the bug only happened at `time = 0`.
1 parent 731ad2a commit 09f9cd2

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

src/common/ttl_map.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ where
184184
Self {
185185
buckets,
186186
data: stage_targets,
187-
time: Arc::new(AtomicU64::new(0)),
187+
// Explicitly initialize `time` to 1 to avoid underflow issues with circular buffer.
188+
time: Arc::new(AtomicU64::new(1)),
188189
gc_scheduler_task: None,
189190
config,
190191
#[cfg(test)]

0 commit comments

Comments
 (0)