-
Notifications
You must be signed in to change notification settings - Fork 397
MSC3982: Limit maximum number of events sent to an AS #3982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
||
- This requires a two way communication between the Homeserver and AS, which presently isn't required. | ||
- This potentially costs more time in negotiation than any efficiency losses from the hardcoded limit. | ||
- Practically speaking, Synapse [has had a limit of 100](https://github.com/matrix-org/synapse/blob/develop/synapse/appservice/scheduler.py#L85-L86) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other homeserver implementations:
- Dendrite has a limit of 50 https://github.com/matrix-org/dendrite/blob/main/appservice/consumers/roomserver.go#L84
- Conduit has a limit of 30 https://gitlab.com/famedly/conduit/-/blob/next/src/service/sending/mod.rs#L159 (I think, it's kinda hard to follow)
Potentially the homeserver and application service could negotiate a maximum event size instead of specifying | ||
one in the spec, however this isn't desirable because: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another reason this is not reasonable is that neither Synapse nor the application service know about any middle boxes between them, so neither of them can possibly know the maximum HTTP request size that is allowed without trial & error.
Synapse seems to implement this already, so I think needs-implementation can go away. |
Rendered
Fixes matrix-org/matrix-spec#1469
Implementations: