fix: tick most RateCounters every 5 seconds instead of every second#1787
Merged
fix: tick most RateCounters every 5 seconds instead of every second#1787
Conversation
filipecabaco
approved these changes
Apr 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates RateCounter to normalize its avg calculation to events per second (independent of the tick interval), and then increases the tick interval for most tenant per-second rate counters from 1s to 5s to reduce per-second work.
Changes:
- Normalize
RateCounter.avgby dividing by the configured tick duration (soavgis always per-second). - Update tenant rate counter configs (joins/events/db events/presence events) to tick every 5 seconds with a 60-second rolling window (12 buckets).
- Adjust and add tests to validate avg normalization and accommodate the new tick interval.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/realtime/rate_counter/rate_counter.ex | Normalizes avg to events/second; updates module docs. |
| lib/realtime/tenants.ex | Changes most tenant per-second counters to 5s tick + 12 buckets. |
| test/realtime/rate_counter/rate_counter_test.exs | Updates existing limit test timing and adds avg normalization tests. |
| test/realtime_web/channels/realtime_channel/presence_handler_test.exs | Increases event counts in rate-limit tests to match 5s tick behavior. |
| test/integration/rt_channel/connection_lifecycle_test.exs | Increases join attempts to trigger join-rate limit under 5s tick. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🎉 This PR is included in version 2.80.12 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
First we fix how
avgis calculated to always reflect per second as it's easier to reason aboutThen we increase the tick for most rate counters to not happen so frequently.
Pros:
Cons:
limit.triggeredflips and enforcement kicks in. With a 1-second tick violations are caught within ~1 second. This is the most significant downside for enforcement.For a tenant sending a constant rate of events, both configurations produce the exact same avg value. The difference only matters during ramp-up, ramp-down, or abuse patterns.