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

Incoming federation transactions stack up indefinitely #9489

Closed
richvdh opened this issue Feb 24, 2021 · 1 comment · Fixed by #9597
Closed

Incoming federation transactions stack up indefinitely #9489

richvdh opened this issue Feb 24, 2021 · 1 comment · Fixed by #9597
Labels
T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@richvdh
Copy link
Member

richvdh commented Feb 24, 2021

Since #8342, we apply the federation ratelimiter after the transaction linearizer. The net effect is that, if we cannot keep up with incoming transactions, we allow them to stack up indefinitely (there can only be one thread inside the transaction linearizer, so we never hit the concurrency limit on the ratelimiter). This can use a lot of memory, and it's not the way matrix federation transactions are meant to work (if the remote gets behind, it should drop messages until it catches up).

It feels like the ratelimter is pretty redundant there. (Admittedly it still has some use as a true rate limiter rather than a concurrency limiter, but that's a bit of an edge-case.)

Should we instead be using a separate ratelimiter dedicated to incoming transactions?
@clokep clokep added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Feb 24, 2021
@richvdh
Copy link
Member Author

richvdh commented Mar 2, 2021

The symptoms of this are that PUT FederationSendServlet requests stack up:

image

richvdh added a commit that referenced this issue Mar 11, 2021
@richvdh
Fonx concurrent transactions
99c9e45 
If more transactions arrive from an origin while we're still processing the
first one, reject them.

Hopefully a quick fix to #9489
richvdh added a commit that referenced this issue Mar 11, 2021
@richvdh
Fonx concurrent transactions
96792a3 
If more transactions arrive from an origin while we're still processing the
first one, reject them.

Hopefully a quick fix to #9489
@richvdh richvdh mentioned this issue Mar 11, 2021
Merged
richvdh added a commit that referenced this issue Mar 11, 2021
@richvdh
Reject concurrent transactions
12743c5 
If more transactions arrive from an origin while we're still processing the
first one, reject them.

Hopefully a quick fix to #9489
richvdh added a commit that referenced this issue Mar 12, 2021
@richvdh
Reject concurrent transactions (#9597)
1e67bff 
If more transactions arrive from an origin while we're still processing the
first one, reject them.

Hopefully a quick fix to #9489
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reject concurrent transactions matrix-org/synapse
2 participants
@clokep @richvdh