matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.83k stars 2.13k forks source link

Filter out auth chain queries that don't exist #16552

Closed realtyem closed 11 months ago

realtyem commented 1 year ago

Short version:

It is kind of a logic fix in that you're doing less work by disregarding the sentinel value of 0. This has two improvements:

  1. Avoiding expanding the chains dictionary unnecessarily.
  2. Avoiding passing more arguments through the SQL engine (and to the database) which we know will result in no matches.
Long version: In `_get_auth_chain_ids_using_cover_index_txn()`: https://github.com/matrix-org/synapse/blob/c14a7de6af0b8a3cbf2e17afca1cab339bc5912d/synapse/storage/databases/main/event_federation.py#L300-L304 Seems to have a small logic problem, where the `max()` can evaluate to `0`. The `chains` dict is passed into the following SQL block: https://github.com/matrix-org/synapse/blob/c14a7de6af0b8a3cbf2e17afca1cab339bc5912d/synapse/storage/databases/main/event_federation.py#L314-L325 where it inhabits the `max_seq` variable. Inside the `event_auth_chains` database table, that column can never be `0`. Filter out that `0` before it gets to the SQL. The temporary metric attached produces results that appear like this(over less than 24 hours): ![auth_chain_condition_hit](https://github.com/matrix-org/synapse/assets/1582365/3f43d502-0537-4fa7-a7fb-01a75b82b656) The metric commit isn't particularly efficient and has no long term relevance, so it will not be included in the final render.

Pull Request Checklist

Signed-off-by: Jason Little realtyem@gmail.com