Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Performance improvements and refactor of Ratelimiter #7595

Merged
merged 33 commits into from
Jun 5, 2020
Merged
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4f715be
Refactor and comment ratelimiting. Set limits in constructor
anoadragon453 May 28, 2020
0e6ee7c
Ratelimiters are instantiated by the HomeServer class
anoadragon453 May 28, 2020
82eac22
Modify servlets to pull Ratelimiters from HomeServer class
anoadragon453 May 28, 2020
a0ef594
Update unittests
anoadragon453 May 28, 2020
6a07c2d
lint
anoadragon453 May 28, 2020
c322ba0
changelog
anoadragon453 May 28, 2020
f6203a6
Make rate_hz and burst_count overridable per-request
anoadragon453 May 29, 2020
1f6156b
Set clock with constructor, store rate_hz per key again
anoadragon453 Jun 1, 2020
c236806
Instantiate Ratelimiters in respective classes
anoadragon453 Jun 1, 2020
470de6e
Use patch for the Ratelimiter in some tests. Set using config in others
anoadragon453 Jun 1, 2020
515a186
Update copyright header
anoadragon453 Jun 1, 2020
87ab836
Remove resolved question
anoadragon453 Jun 1, 2020
56c52a5
lint
anoadragon453 Jun 1, 2020
2d7e087
lint, mypy
anoadragon453 Jun 1, 2020
a566b46
Remove unittest.DEBUG statement
anoadragon453 Jun 1, 2020
41c7288
Update changelog.d/7595.misc
anoadragon453 Jun 2, 2020
58d4919
Remove erroneous print statement
anoadragon453 Jun 2, 2020
aa1f4c3
Merge branch 'anoa/ratelimit_config_perf' of github.com:matrix-org/sy…
anoadragon453 Jun 2, 2020
39b484b
Move update after optional method arguments
anoadragon453 Jun 2, 2020
d727bed
Make it obvious that time_now_s is just for testing
anoadragon453 Jun 2, 2020
9f76a8d
Update ratelimiter calling methods and tests
anoadragon453 Jun 2, 2020
8867900
No need to re-check for None in can_do_action
anoadragon453 Jun 2, 2020
ef7383f
time_now_s is used in ratelimit
anoadragon453 Jun 3, 2020
189c01b
Comment changes revolving around time_allowed
anoadragon453 Jun 3, 2020
4a88edb
Fix missed call to self.rate_hz
anoadragon453 Jun 3, 2020
14a0af5
Test Ratelimiter ratelimit method and param overrides
anoadragon453 Jun 3, 2020
c145c81
Back out some changes.
clokep Jun 4, 2020
12b4d47
Do not specify ratelimiters in tests when unnecessary.
clokep Jun 4, 2020
d84d779
Update timestamp comment
anoadragon453 Jun 4, 2020
45a7791
Clean up Exception raising assertion
anoadragon453 Jun 4, 2020
3899589
Clean up and split out tests
anoadragon453 Jun 4, 2020
08c5114
Remove _ = style
anoadragon453 Jun 4, 2020
9beee5f
Merge branch 'anoa/ratelimit_config_perf' of github.com:matrix-org/sy…
anoadragon453 Jun 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions synapse/api/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def can_do_action(
A tuple containing:
* A bool indicating if they can perform the action now
* The time in seconds of when it can next be performed.
-1 if a rate_hz has not been defined for this Ratelimiter
-1 if rate_hz is less than or equal to zero
"""
# Override default values if set
time_now_s = _time_now_s if _time_now_s is not None else self.clock.time()
Expand Down Expand Up @@ -100,15 +100,17 @@ def can_do_action(
if update:
self.actions[key] = (action_count, time_start, rate_hz)

# Figure out the time when an action can be performed again
if self.rate_hz > 0:
# Find out when the count of existing actions expires
time_allowed = time_start + (action_count - burst_count + 1) / rate_hz

# Don't give back a time in the past
if time_allowed < time_now_s:
time_allowed = time_now_s
else:
# This does not apply
# XXX: Why is this -1? This seems to only be used in
# self.ratelimit. I guess so that clients get a time in the past and don't
# feel afraid to try again immediately
Copy link
Member Author

@anoadragon453 anoadragon453 Jun 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'm tempted to resolve this, but I don't want to change the behaviour of Ratelimiter in this PR any more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this sounds like a follow-up!

time_allowed = -1

return allowed, time_allowed
Expand Down