element-hq / element-hq/synapse

Redundant `highlight` column on `event_push_actions_stream_highlight_index` index seems to defeat query planner

Open
#14,289 0 comments 0 reactions 0 assignees View on GitHub
O-Uncommon S-Minor T-Defect
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#14289](https://github.com/matrix-org/synapse/issues/14289).

---

We are sometimes seeing slow query performance on `event_push_actions`, though we only seem to notice problems when something tips it over the edge (e.g. an autovacuum(?)).
Here is an issue I noticed when examining the query. I suspect small homeservers are unlikely to see much impact from this problem.

This is the index in question:
`"event_push_actions_stream_highlight_index" btree (highlight, stream_ordering) WHERE highlight = 0`

This is the SELECT form of a DELETE that has frequently been taking 5~6 seconds to execute.
```sql
matrix=> EXPLAIN SELECT FROM event_push_actions WHERE stream_ordering <= 3380594925 AND highlight = 0;
QUERY PLAN
═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Index Only Scan using event_push_actions_stream_highlight_index on event_push_actions (cost=0.56..11990944.46 rows=13892710 width=0)
Index Cond: (stream_ordering <= '3380594925'::bigint)
(2 rows)

Time: 0.665 ms
matrix=> EXPLAIN ANALYZE SELECT FROM event_push_actions WHERE stream_ordering <= 3380594925 AND highlight = 0;
QUERY PLAN
═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Index Only Scan using event_push_actions_stream_highlight_index on event_push_actions (cost=0.56..11990944.46 rows=13892710 width=0) (actual time=6389.624..6389.626 rows=0 loops=1)
Index Cond: (stream_ordering <= '3380594925'::bigint)
Heap Fetches: 4391136
Planning time: 0.127 ms
Execution time: 6389.656 ms
(5 rows)

Time: 6390.290 ms (00:06.390)
```
(N.B. This query returns 0 — no rows)

My observations:
- `btree (highlight, stream_ordering) WHERE highlight = 0` is generally poor — one of the `highlight` columns in the index entries or in the WHERE predicate is redundant. This is a waste of space. The `highlight` column in the prefix of the b-tree entry should be removed.
- The query based on this index is performing *an awful lot* of heap fetches for something that, as a human, we can reason about to require none of them at all to check the WHERE predicate of the query.
- The `Index Cond` seems wrong: `(stream_ordering <= '3380594925'::bigint)` — it doesn't mention `highlight`. Since the condition without `highlight` doesn't cover the prefix of the B-Tree's entries, then Postgres can't just scan an interesting segment of the index.

My hypothesis is that having `highlight` defined both as a prefix of the B-Tree indices and as a WHERE predicate on the index is somehow defeating Postgres's query planner to not include `highlight` in the index condition. This is technically a Postgres bug, I suppose(?), but it's a poor index anyway.

# Alternative indices
On my personal homeserver I mucked about a bit and tried some new indices (after marking the old one as invalid to disable it):

## Remove the WHERE predicate
```
synapse=# create index on event_push_actions (highlight, stream_ordering);
CREATE INDEX
synapse=# EXPLAIN ANALYZE SELECT COUNT(1) FROM event_push_actions WHERE stream_ordering <= 3325 AND highlight = 0;
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=6.06..6.07 rows=1 width=8) (actual time=0.078..0.079 rows=1 loops=1)
-> Index Only Scan using event_push_actions_highlight_stream_ordering_idx on event_push_actions (cost=0.28..6.05 rows=1 width=0) (actual time=0.073..0.074 rows=0 loops=1)
Index Cond: ((highlight = 0) AND (stream_ordering <= 3325))
Heap Fetches: 0
Planning Time: 0.695 ms
Execution Time: 0.124 ms
(6 rows)

```

Note how the index condition includes `(highlight = 0)`.

N.B. This option consumes more disk space. I only include it to demonstrate what the Index Condition 'should' look like.

## Remove `highlight` from the index entry
```
create index on event_push_actions (stream_ordering) WHERE highlight = 0;
```

This is probably preferred. Unscientific testing on my homeserver suggests that Postgres prefers to use this index, whereas it would sometimes prefer to do a seq scan than use the old index.

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.