matrix-org / matrix-org/matrix-spec
Pre-filtering load limits are not discussed in the spec
- Dominant language
- HTML
- Stars
- 330
- Forks
- 150
- Avg merge
- 2h 21m
- Merged PRs (30d)
- 3
Description
**Link to problem area**:
I think this should be discussed with the [filter schema](https://spec.matrix.org/v1.11/client-server-api/#api-endpoints), and possibly in endpoints that support filtering:
- [`GET /client/v3/rooms/{roomId}/messages`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3roomsroomidmessages)
- [`GET /client/v3/sync`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3roomsroomidmessages)
- [`GET /client/v3/rooms/{roomId}/context/{eventId}`](https://spec.matrix.org/v1.11/client-server-api/#get_matrixclientv3roomsroomidcontexteventid)
- [`POST /client/v3/search`](https://spec.matrix.org/v1.11/client-server-api/#post_matrixclientv3search)
**Issue**
In a practical filtering implementation, the server has to handle the case where a filter rejects a significant fraction of events. In this situation, finding `limit` accepted events may involve loading a very large number of rejected events from the database first. This is a DoS vector, and also may result in unacceptably long response latency.
The way current implementations deal with this is imposing a separate "load limit" on the number of events that will be loaded from the database pre-filtering. If the server hits the load limit, it will return the accepted events that it has found so far even if it is less than `limit` from the filter.
In synapse, load limit is set to `max(limit * FACTOR, 10)`, where `FACTOR` is a admin-configurable value that defaults to `2`. We intend to use the same formula in grapevine's filtering implementation.
The load limit situation is not discussed directly anywhere in the spec. The text for the filter parameter on the `/client/v3/rooms/{roomId}/context/{eventId}` endpoint does state that
> The filter may be applied before or/and after the limit parameter - whichever the homeserver prefers.
Which is similar, although not identical to the load limit behavior in synapse and grapevine. This text states that the server may choose to limit the number of events loaded from the database pre-filtering, but implies that this load limit should be equal to the `limit` parameter, rather than allowing the load limit to be greater than `limit` the way that synapse and grapevine do.
I think "the load limit is an implementation detail and doesn't belong in the spec" is a reasonable position, but believe that it should be mentioned in the spec because it has some non-obvious consequences for sync gaps and `/messages` pagination and because client developers may (incorrectly) expect that `limit = N` means that they will get `N` allowed events if `N` are available.
Contributor guide
Research direction
Start with the linked filter schema and the four endpoint sections: /messages, /sync, /context, and /search. Compare their existing limit and filtering language, then document the pre-filter load-limit behavior and its consequences for pagination and sync gaps, with wording that clarifies what clients can expect.
Written by the indexing model from the issue text.
Assessment
- Domain
- api, documentation
- Issue type
- Documentation
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100