element-hq / element-hq/synapse

Specify concurrency per background job type

Open
#18,006 1 comment 0 reactions 0 assignees View on GitHub
T-Enhancement
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.