Skip to content

Commit f6d9b25

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 f6d9b25

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

src/common/ttl_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ where
184184
Self {
185185
buckets,
186186
data: stage_targets,
187-
time: Arc::new(AtomicU64::new(0)),
187+
time: Arc::new(AtomicU64::new(1)),
188188
gc_scheduler_task: None,
189189
config,
190190
#[cfg(test)]

0 commit comments

Comments
 (0)