`InMemoryStore` grows unbounded
While we do have a TtlCache<Counter, i64>, the Set of Counter in limits_for_namespace: RwLock<HashMap<Namespace, HashMap<Limit, HashSet<Counter>>>> will grow unbounded. The TtlCache<Counter, i64>, limited to 1000 entries by default, will evict expired entries (when at capacity I guess?), the Set of Counters in our "main" datastructure is unbounded. Not necessarily a big issue, as it "only" will grow for "qualified" (probably not the best name) Limits (i.e. ones that have a counter per some arbitrary variable).
If we use only a single set of counters (i.e. the one in the main datastructure), we and once we have a non-blocking way of maintaining these, we probably should implement some eviction/expiry mechanism to reclaim memory of these counters.
I've debugged this using this test below… while the rate_limiter.get_counters("foo".to_owned()).unwrap().len() will return 0, going deeper in the InMemoryStore you'd see that the value of the <Limit, HashSet<Counter>> mapping for that Limit is a HashSet of size a 1000. It "merely" gets cleaned up in using the TTL of the cache in:
for counter in self.counters_in_namespace(namespace) { // 1000 counters
if let Some(counter_val) = self.counters.read().unwrap().get(&counter) { // which are all expired here
let mut counter_with_val = counter.clone();
counter_with_val.set_remaining(*counter_val);
res.insert(counter_with_val);
}
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::thread;
use std::time::Duration;
use crate::{Limit, RateLimiter};
#[test]
fn test_leak() {
let rate_limiter = RateLimiter::default();
let nothing: Vec<String> = vec![];
let limit = Limit::new(
"foo",
2,
1,
nothing.clone(),
vec!["id"],
);
rate_limiter.add_limit(&limit).unwrap();
for o in 0..3 {
for l in 0..3 {
for i in 0..1000 { // grow the store even bigger, by making this >1000...
let mut map = HashMap::new();
map.insert("id".to_owned(), i.to_string());
assert_eq!(l == 2, rate_limiter // ... but requires removing this assertion, as limits won't be enforced, despite we holding a complete datastructure to be able to do so... but the TtlCache being used for expired counters, when at capacity, it considers these counters "expired".
.check_rate_limited_and_update("foo", &map, 1)
.unwrap());
}
}
if o == 2 {
thread::sleep(Duration::from_secs(2));
} else {
thread::sleep(Duration::from_secs(1));
}
}
println!("Counters: {}", rate_limiter.get_counters("foo".to_owned()).unwrap().len());
println!("Counters: {}", rate_limiter.get_counters("foo".to_owned()).unwrap().len());
}
}