Skip to content

refactor: keyspace helpers for expiry, collection cleanup, and error strings #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kacy merged 4 commits into from
Feb 12, 2026
Merged

refactor: keyspace helpers for expiry, collection cleanup, and error strings #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8062b75
refactor: add expiry count helpers to keyspace
kacy Feb 12, 2026
d079229
refactor: extract cleanup_after_remove helper
kacy Feb 12, 2026
4c1da83
refactor: consolidate remaining_secs/remaining_ms in time.rs
kacy Feb 12, 2026
fcb881a
refactor: shared error message constants for WRONGTYPE and OOM
kacy Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 54 additions & 96 deletions crates/ember-core/src/keyspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ use crate::time;
use crate::types::sorted_set::{SortedSet, ZAddFlags};
use crate::types::{self, normalize_range, Value};

const WRONGTYPE_MSG: &str = "WRONGTYPE Operation against a key holding the wrong kind of value";
const OOM_MSG: &str = "OOM command not allowed when used memory > 'maxmemory'";

/// Error returned when a command is used against a key holding the wrong type.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct WrongType;

impl std::fmt::Display for WrongType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"WRONGTYPE Operation against a key holding the wrong kind of value"
)
write!(f, "{WRONGTYPE_MSG}")
}
}

Expand Down Expand Up @@ -66,15 +66,10 @@ pub enum IncrError {
impl std::fmt::Display for IncrError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
IncrError::WrongType => write!(
f,
"WRONGTYPE Operation against a key holding the wrong kind of value"
),
IncrError::WrongType => write!(f, "{WRONGTYPE_MSG}"),
IncrError::NotAnInteger => write!(f, "ERR value is not an integer or out of range"),
IncrError::Overflow => write!(f, "ERR increment or decrement would overflow"),
IncrError::OutOfMemory => {
write!(f, "OOM command not allowed when used memory > 'maxmemory'")
}
IncrError::OutOfMemory => write!(f, "{OOM_MSG}"),
}
}
}
Expand All @@ -97,19 +92,12 @@ pub enum IncrFloatError {
impl std::fmt::Display for IncrFloatError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
IncrFloatError::WrongType => write!(
f,
"WRONGTYPE Operation against a key holding the wrong kind of value"
),
IncrFloatError::NotAFloat => {
write!(f, "ERR value is not a valid float")
}
IncrFloatError::WrongType => write!(f, "{WRONGTYPE_MSG}"),
IncrFloatError::NotAFloat => write!(f, "ERR value is not a valid float"),
IncrFloatError::NanOrInfinity => {
write!(f, "ERR increment would produce NaN or Infinity")
}
IncrFloatError::OutOfMemory => {
write!(f, "OOM command not allowed when used memory > 'maxmemory'")
}
IncrFloatError::OutOfMemory => write!(f, "{OOM_MSG}"),
}
}
}
Expand Down Expand Up @@ -327,6 +315,39 @@ impl Keyspace {
self.drop_handle = Some(handle);
}

/// Decrements the expiry count if the entry had a TTL set.
fn decrement_expiry_if_set(&mut self, entry: &Entry) {
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
}

/// Cleans up after removing an element from a collection (list, sorted
/// set, hash, or set). If the collection is now empty, removes the key
/// entirely and adjusts memory/expiry tracking. Otherwise just adjusts
/// the memory delta.
fn cleanup_after_remove(&mut self, key: &str, old_size: usize, is_empty: bool) {
if is_empty {
if let Some(removed) = self.entries.remove(key) {
self.decrement_expiry_if_set(&removed);
}
self.memory.remove_with_size(old_size);
} else {
let new_size = memory::entry_size(key, &self.entries[key].value);
self.memory.adjust(old_size, new_size);
}
}

/// Adjusts the expiry count when replacing an entry whose TTL status
/// may have changed (e.g. SET overwriting an existing key).
fn adjust_expiry_count(&mut self, had_expiry: bool, has_expiry: bool) {
match (had_expiry, has_expiry) {
(false, true) => self.expiry_count += 1,
(true, false) => self.expiry_count = self.expiry_count.saturating_sub(1),
_ => {}
}
}

/// Retrieves the string value for `key`, or `None` if missing/expired.
///
/// Returns `Err(WrongType)` if the key holds a non-string value.
Expand Down Expand Up @@ -386,13 +407,7 @@ impl Keyspace {

if let Some(old_entry) = self.entries.get(&key) {
self.memory.replace(&key, &old_entry.value, &new_value);
// adjust expiry count if the TTL status changed
let had_expiry = old_entry.expires_at_ms != 0;
match (had_expiry, has_expiry) {
(false, true) => self.expiry_count += 1,
(true, false) => self.expiry_count = self.expiry_count.saturating_sub(1),
_ => {}
}
self.adjust_expiry_count(old_entry.expires_at_ms != 0, has_expiry);
} else {
self.memory.add(&key, &new_value);
if has_expiry {
Expand Down Expand Up @@ -428,9 +443,7 @@ impl Keyspace {
if let Some(key) = victim {
if let Some(entry) = self.entries.remove(&key) {
self.memory.remove(&key, &entry.value);
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&entry);
self.evicted_total += 1;
self.defer_drop(entry.value);
return true;
Expand Down Expand Up @@ -474,9 +487,7 @@ impl Keyspace {
}
if let Some(entry) = self.entries.remove(key) {
self.memory.remove(key, &entry.value);
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&entry);
self.defer_drop(entry.value);
true
} else {
Expand All @@ -495,9 +506,7 @@ impl Keyspace {
}
if let Some(entry) = self.entries.remove(key) {
self.memory.remove(key, &entry.value);
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&entry);
// always defer for UNLINK, regardless of value size
if let Some(ref handle) = self.drop_handle {
handle.defer_value(entry.value);
Expand Down Expand Up @@ -808,16 +817,12 @@ impl Keyspace {

// update memory tracking for old key removal
self.memory.remove(key, &entry.value);
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&entry);

// remove destination if it exists
if let Some(old_dest) = self.entries.remove(newkey) {
self.memory.remove(newkey, &old_dest.value);
if old_dest.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&old_dest);
}

// re-insert with the new key name, preserving value and expiry
Expand Down Expand Up @@ -933,12 +938,7 @@ impl Keyspace {
// if replacing an existing entry, adjust memory tracking
if let Some(old) = self.entries.get(&key) {
self.memory.replace(&key, &old.value, &value);
let had_expiry = old.expires_at_ms != 0;
match (had_expiry, has_expiry) {
(false, true) => self.expiry_count += 1,
(true, false) => self.expiry_count = self.expiry_count.saturating_sub(1),
_ => {}
}
self.adjust_expiry_count(old.expires_at_ms != 0, has_expiry);
} else {
self.memory.add(&key, &value);
if has_expiry {
Expand Down Expand Up @@ -1124,19 +1124,8 @@ impl Keyspace {
};
entry.touch();

// check if list is now empty — if so, delete the key entirely
let is_empty = matches!(&entry.value, Value::List(d) if d.is_empty());
if is_empty {
let removed = self.entries.remove(key).expect("verified above");
// use remove_with_size since the value was already mutated
self.memory.remove_with_size(old_entry_size);
if removed.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
} else {
let new_entry_size = memory::entry_size(key, &self.entries[key].value);
self.memory.adjust(old_entry_size, new_entry_size);
}
self.cleanup_after_remove(key, old_entry_size, is_empty);

Ok(popped)
}
Expand Down Expand Up @@ -1255,16 +1244,7 @@ impl Keyspace {
entry.touch();

let is_empty = matches!(&entry.value, Value::SortedSet(ss) if ss.is_empty());
if is_empty {
let removed_entry = self.entries.remove(key).expect("verified above");
self.memory.remove_with_size(old_entry_size);
if removed_entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
} else {
let new_entry_size = memory::entry_size(key, &self.entries[key].value);
self.memory.adjust(old_entry_size, new_entry_size);
}
self.cleanup_after_remove(key, old_entry_size, is_empty);

Ok(removed)
}
Expand Down Expand Up @@ -1494,17 +1474,7 @@ impl Keyspace {
false
};

if is_empty {
if let Some(removed_entry) = self.entries.remove(key) {
if removed_entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
}
self.memory.remove_with_size(old_entry_size);
} else {
let new_entry_size = memory::entry_size(key, &self.entries[key].value);
self.memory.adjust(old_entry_size, new_entry_size);
}
self.cleanup_after_remove(key, old_entry_size, is_empty);

Ok(removed)
}
Expand Down Expand Up @@ -1759,17 +1729,7 @@ impl Keyspace {
false
};

if is_empty {
if let Some(removed_entry) = self.entries.remove(key) {
if removed_entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
}
self.memory.remove_with_size(old_entry_size);
} else {
let new_entry_size = memory::entry_size(key, &self.entries[key].value);
self.memory.adjust(old_entry_size, new_entry_size);
}
self.cleanup_after_remove(key, old_entry_size, is_empty);

Ok(removed)
}
Expand Down Expand Up @@ -1864,9 +1824,7 @@ impl Keyspace {
if expired {
if let Some(entry) = self.entries.remove(key) {
self.memory.remove(key, &entry.value);
if entry.expires_at_ms != 0 {
self.expiry_count = self.expiry_count.saturating_sub(1);
}
self.decrement_expiry_if_set(&entry);
self.expired_total += 1;
self.defer_drop(entry.value);
}
Expand Down
15 changes: 8 additions & 7 deletions crates/ember-core/src/time.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,22 @@ pub fn expiry_from_duration(ttl: Option<std::time::Duration>) -> u64 {
/// Returns remaining TTL in seconds, or None if no expiry.
#[inline]
pub fn remaining_secs(expires_at_ms: u64) -> Option<u64> {
if expires_at_ms == NO_EXPIRY {
None
} else {
let now = now_ms();
Some(expires_at_ms.saturating_sub(now) / 1000)
}
remaining(expires_at_ms, 1000)
}

/// Returns remaining TTL in milliseconds, or None if no expiry.
#[inline]
pub fn remaining_ms(expires_at_ms: u64) -> Option<u64> {
remaining(expires_at_ms, 1)
}

/// Shared implementation for remaining TTL with a unit divisor.
#[inline]
fn remaining(expires_at_ms: u64, divisor: u64) -> Option<u64> {
if expires_at_ms == NO_EXPIRY {
None
} else {
let now = now_ms();
Some(expires_at_ms.saturating_sub(now))
Some(expires_at_ms.saturating_sub(now) / divisor)
}
}