element-hq / element-hq/synapse
Don't waste time processing backfill events that we never show to the client
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
This issue has been migrated from [#15632](https://github.com/matrix-org/synapse/issues/15632).
---
*As discovered while working on https://github.com/matrix-org/synapse/pull/15617 and a natural extension of https://github.com/matrix-org/synapse/pull/15585 to process previously failed backfill events in the background (follow-up to https://github.com/matrix-org/synapse/issues/13623),*
It turns out, when we make a `/messages` request and trigger a backfill, Synapse will happily waste time trying hard to process events ([`_process_pulled_events`](https://github.com/matrix-org/synapse/blob/1e89976b268c296e1fd8fface36ade29c0354254/synapse/handlers/federation_event.py#L819-L898)) in the foreground that we end up never showing to clients. The one particularly bad culprit is the `org.matrix.dummy_event` which Synapse automatically puts in the DAG to resolve outstanding forward extremities. Since it has 10x random `prev_events`, it takes a while to fetch the state for each `prev_event` and persist. We determine whether some events should be shown to clients in the `_check_filter_send_to_client(...)` function
https://github.com/matrix-org/synapse/blob/ad50510a06d035a674f0eeed5db5dd3060bc0b1c/synapse/visibility.py#L482-L519
`org.matrix.dummy_event` can be particularly bad because they usually include a lot of disparate `prev_events` which take a while for us to work out the state for (long [`_get_state_groups_from_groups`](https://github.com/matrix-org/synapse/blob/41b9def9f2c02118796e147f63abf23bc2d7dc04/synapse/storage/databases/state/bg_updates.py#L86-L250) times).
### Potential solution
We should use this same `_check_filter_send_to_client(...)` criteria when determining whether we should backfill a given event in the background or foreground.
We should be mindful an event that is filtered from the client, might still be used as a `prev_event` from another event in the backfill response that we will show to clients. And if we kick off the function to process `org.matrix.dummy_event` in the background, while the other event in the foreground, we will probably end up duplicating the work depending on the race.
We don't really have these same race -> duplicate work concerns with https://github.com/matrix-org/synapse/pull/15585 (events which previously failed to backfill) because it's a "fool me once" sort of situation and they are already potentially disconnected from what we were trying to backfill anyway given they failed before.
---
Also of interest, `filter_events_for_server(...)`
Contributor guide
Assessment
This issue has not been assessed yet.