pingora-limits - Rate Estimator hashes & slots configuration

[PawelJastrzebski]

What is the problem your feature solves, or the need it fulfills?

pingora-limits Unable to set custom hashes and slots for adjusting false positive ratio while constructing Rate

hashes: h, slots: n
False positive ratio is 1/(n^h)

default values in "constructor" fn.

// see inflight module for the meaning for these numbers
const HASHES: usize = 4;
const SLOTS: usize = 1024; // This value can be lower if interval is short (key cardinality is low)

impl Rate {
    /// Create a new `Rate` with the given interval.
    pub fn new(interval: std::time::Duration) -> Self {
        Rate {
            red_slot: Estimator::new(HASHES, SLOTS),
            blue_slot: Estimator::new(HASHES, SLOTS),
            red_or_blue: AtomicBool::new(true),
            start: Instant::now(),
            reset_interval_ms: interval.as_millis() as u64, // should be small not to overflow
            last_reset_time: AtomicU64::new(0),
        }
    }

Describe the solution you'd like

new "constructor" fn new_custom(interval, hashes, slots)

impl Rate {
    /// Create a new `Rate` with the given interval.
    pub fn new(interval: std::time::Duration) -> Self {
        Rate::new_custom(interval, HASHES, SLOTS)
    }

    pub fn new_custom(interval: std::time::Duration, hashes: usize, slots: usize) -> Self {
        Rate {
            red_slot: Estimator::new(hashes, slots),
            blue_slot: Estimator::new(hashes, slots),
            red_or_blue: AtomicBool::new(true),
            start: Instant::now(),
            reset_interval_ms: interval.as_millis() as u64, // should be small not to overflow
            last_reset_time: AtomicU64::new(0),
        }
    }

Describe alternatives you've considered

Adding builder like function instead of new constructor.

     pub fn custom_estimator(self, hashes: usize, slots: usize) -> Self {
        Self {
            red_slot: Estimator::new(hashes, slots),
            blue_slot: Estimator::new(hashes, slots),
            ..self
        }
    }

Additional context

Source file: https://docs.rs/pingora-limits/0.3.0/src/pingora_limits/rate.rs.html#43

[PawelJastrzebski]

@drcaramelsyrup thx for review.

I would be happy to make PR with that change, but I'm not sure what implementation style is appropriate. Could you guide me in what direction should I go?

[drcaramelsyrup]

Hi there, thanks @PawelJastrzebski . IMO the new constructor works fine.