Skip to content

Commit

Permalink
Don't overcharge tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
dcadenas committed Sep 27, 2024
1 parent 87d05f1 commit 5a5d3c0
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 11 deletions.
3 changes: 1 addition & 2 deletions src/domain/followee_notification_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ impl FolloweeNotificationFactory {
.collect();

let tokens_needed = messages.len() as f64;

self.rate_limiter.overcharge(tokens_needed);
self.rate_limiter.consume(tokens_needed);

if let Some(follow_change) = followers.first() {
info!(
Expand Down
10 changes: 5 additions & 5 deletions src/domain/notification_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,13 @@ mod tests {
let messages = notification_factory.flush();
assert_eq!(messages.len(), 0);

// And we finally hit the min period
// And we finally hit the min period so we get one token
advance(Duration::from_secs(6u64)).await;
let messages = notification_factory.flush();

// But it's retained, there are not tokens
assert_eq!(messages.len(), 0);
assert_eq!(notification_factory.follow_changes_len(), 1);
// The token is used by the last change
assert_eq!(messages.len(), 1);
assert_eq!(notification_factory.follow_changes_len(), 0);

// And another change arrives
insert_new_follower(&mut notification_factory, followee);
Expand All @@ -351,7 +351,7 @@ mod tests {
let messages = notification_factory.flush();
// The new one is flushed, the old one is retained
assert_eq!(messages.len(), 1);
assert_eq!(notification_factory.follow_changes_len(), 2);
assert_eq!(notification_factory.follow_changes_len(), 1);

advance(Duration::from_secs(min_seconds_between_messages as u64 + 1)).await;
let messages = notification_factory.flush();
Expand Down
11 changes: 7 additions & 4 deletions src/rate_limiter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,23 @@ impl RateLimiter {
self.tokens >= tokens_needed
}

/// Attempts to consume the specified number of tokens.
/// Consumes the specified number of tokens.
pub fn consume(&mut self, tokens_needed: f64) -> bool {
self.refill_tokens();

if self.tokens >= tokens_needed {
self.tokens -= tokens_needed;
true
} else {
self.tokens = 0.0;
false
}
}

/// Consumes tokens regardless of availability (can result in negative token count).
/// Consumes the specified number of tokens, allows deficit.
pub fn overcharge(&mut self, tokens_needed: f64) {
self.refill_tokens();

self.tokens -= tokens_needed;

if self.tokens < -self.max_negative_tokens {
Expand Down Expand Up @@ -116,8 +119,8 @@ mod tests {
// Attempt to consume more tokens than available
let result = rate_limiter.consume(15.0);
assert!(!result);
// Tokens should remain unchanged since consume failed
assert_eq!(rate_limiter.tokens, capacity);
// Should have consumed all it could
assert_eq!(rate_limiter.tokens, 0.0);
}

#[tokio::test]
Expand Down

0 comments on commit 5a5d3c0

Please sign in to comment.