Conversation
ba2649a to
e950099
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3184 +/- ##
========================================
+ Coverage 1.36% 2.84% +1.48%
========================================
Files 360 361 +1
Lines 13815 13856 +41
========================================
+ Hits 188 394 +206
+ Misses 13627 13462 -165 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cc0d4e7 to
684275b
Compare
rorymckinley
left a comment
There was a problem hiding this comment.
@jyeshe I always enjoy PRs that force me to do some learning :).
Nice code - just one question to make sure that have not missed something: Would I be correct in saying that the distributed rate limiter is disabled in Lightning, but will be enabled in the billing app?
Also, I did not run the tests as I have an error with the Sentry library dependency (I guess this is what happens when I don't refresh the repo for two weeks :D) and I do not want to block the PR while I am figuring it out. If you feel it is important that I do run these tests please let me know and I will do so.
Hey @rorymckinley, that's correct. So far there is no requirement to enable it on Lightning. I think we are fine with the local tests, I am willing to see it more on a K8S cluster. I am optimistic with the results on a low latency private network. |
28cfd61 to
a88a109
Compare
a88a109 to
1271253
Compare
Description
This PR provides a rate limiter based on distributed process that works as token bucket. The token bucket has a capacity (credits for requests) and a refill rate. For example if the capacity is 10 and refill per second is 2, the user can make 10 requests and after that 2 requests per second. When stops to make requests, it refills up to 10 credits and can make 10 requests in 1 second again.
It uses CRDTs via Horde library only to replicate the process and by doing so it doesn't need to replicate the state of the rate limiting cache.
Closes #3185
Validation steps
iex --sname node1@localhost --cookie hordecookie -S mix phx.serverRTM=false PORT=4001 iex --sname node2@localhost --cookie hordecookie -S mix phx.serverLightning.DistributedRateLimiter.inspect_table()on both iex shells and they shall show the same process and node.Additional notes for the reviewer
The rate limiting cache state is transient and considered negligible in case of a network split in a way that after the process is recreated in another node it doesn't care about the previous state. If nothing happened, no network split, with the proposed configuration, after 5 seconds the credits would have been completed restored anyways for all projects.
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Pre-submission checklist
:owner,:admin,:editor,:viewer)