element-hq / element-hq/synapse
Specify concurrency per background job type
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
### Proposal
In our production clusters, we're occasionally seeing degraded performance on some of our hosts. This has been traced back to 5 concurrent room purges happening at once, which end up starving the database for any other tasks that need doing. These room purges are triggered by the [`forgotten_room_retention_period`](https://element-hq.github.io/synapse/latest/usage/configuration/config_documentation.html#forgotten_room_retention_period) config option being enabled.
This option works by scheduling the purge when a user forgets the room (if they're the last one to do so):
https://github.com/element-hq/synapse/blob/ecbc0b740cc881c6094ef6ed22ab9d9e7b65e322/synapse/handlers/room_member.py#L329-L341
The `TaskScheduler` was chosen for this so that if Synapse restarts during the purge, the room isn't left in a half-deleted state, as the task will be resumed again on startup. We have a few different types of jobs that can be queued using the `TaskScheduler`:
* Purging an entire room
* Purging history from a room
* Deleting old to-device messages
* Redacting the events of a given user in a set of given rooms
Completing one of each of these tasks varies wildly in terms of how many resources it takes. For instance, purging a large room with lots of message history and state events will take much longer than deleting old to-device messages for a fairly inactive user. You probably want a low concurrency for the former, and a high concurrency for the latter.
The global task concurrency is set here:
https://github.com/element-hq/synapse/blob/ecbc0b740cc881c6094ef6ed22ab9d9e7b65e322/synapse/util/task_scheduler.py#L85-L86
To work around this problem, we could introduce a hardcoded concurrency per-task type. Perhaps 1 or 2 for purge room, and something much higher for to-device messages. Perhaps we could still have an overall concurrency limit, but tasks of a certain type are prevented from starting if its already at that type's limit (and a different type is allowed to run instead).
For this, I think we would need to consolidate where task types are defined inside of [`synapse/util/task_scheduler.py`](synapse/util/task_scheduler.py), as right now they're simply defined on the fly in each file. We can then assign each type a concurrency limit.
An additional benefit is that this will allow smaller tasks to still run while continuing to work on large ones slowly.
### Potential Issues
- This is still a fairly coarse approach. For instance, purging small rooms will be much quicker than purging large ones. A better solution would require us to estimate a task's resource impact. Perhaps we could do that in the future, but this proposal is at least a step in the right direction.
- This is still a hardcoded solution. Future work could involve allowing each limit to be configurable.
Contributor guide
Assessment
This issue has not been assessed yet.