Fix cross-worker ratelimiting #16558
Conversation
Co-authored-by: Patrick Cloke <[email protected]>
clokep
left a comment
clokep
left a comment
There was a problem hiding this comment.
Overall seems reasonable, but a couple sanity checks.
synapse/handlers/message.py
Outdated
| if writer_instance != self._instance_name: | ||
| # Ratelimit before sending to the other event persister, to | ||
| # ensure that we correctly have ratelimits on both the event | ||
| # creators and event persiters. |
There was a problem hiding this comment.
| # creators and event persiters. | |
| # creators and event persisters. |
There was a problem hiding this comment.
If there are multiple creators going to the same persister (Is that possible?) than we might pass this rate check, but fail the one on the persister.
I think the opposite isn't true since persisters are sharded by room so we either have a 1-to-1 mapping or a many-to-1 mapping? But the creater could fill the bucket faster if for some reason the send_events below fails (e.g. the persister is offline for a period?)
I guess restarts would also make these go out of sync?
There was a problem hiding this comment.
Yeah, these can totally get out of sync. However, if they do get out of sync then the worst that is going to happen is that you're being ratelimited when you're already over the threshold, so 🤷
Co-authored-by: Patrick Cloke <[email protected]>
c.f. #16481