This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Optimize filter_events_for_client for faster /messages - v1
#14494
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MadLittleMods
added
T-Enhancement
MadLittleMods
commented
Nov 19, 2022
MadLittleMods
commented
Nov 19, 2022
MadLittleMods
commented
Nov 19, 2022
| @@ -130,9 +142,10 @@ def allowed(event: EventBase) -> Optional[EventBase]: | |||
| sender_erased=erased_senders.get(event.sender, False), | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
Why does the state_groups table exist?
$ psql synapse
# \d+ state_groups
Table "public.state_groups"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
----------+--------+-----------+----------+---------+----------+--------------+-------------
id | bigint | | not null | | plain | |
room_id | text | | not null | | extended | |
event_id | text | | not null | | extended | |It seems like a subset of state_groups_state
$ psql synapse
# \d+ state_groups_state
Table "public.state_groups_state"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
-------------+--------+-----------+----------+---------+----------+--------------+-------------
state_group | bigint | | not null | | plain | |
room_id | text | | not null | | extended | |
type | text | | not null | | extended | |
state_key | text | | not null | | extended | |
event_id | text | | not null | | extended | |
MadLittleMods
commented
Nov 19, 2022
MadLittleMods
commented
Nov 22, 2022
Comment on lines
306
to
325
| WITH RECURSIVE sgs(state_group) AS ( | ||
| VALUES(?::bigint) | ||
| UNION ALL | ||
| SELECT prev_state_group FROM state_group_edges e, sgs s | ||
| WHERE s.state_group = e.state_group | ||
| ) | ||
| SELECT | ||
| type, state_key, event_id | ||
| FROM state_groups_state | ||
| WHERE | ||
| state_group IN ( | ||
| SELECT state_group FROM sgs | ||
| ) | ||
| AND (type = ? AND state_key = ?) | ||
| ORDER BY | ||
| type, | ||
| state_key, | ||
| -- Use the lastest state in the chain (highest numbered state_group in the chain) | ||
| state_group DESC | ||
| LIMIT 1 |
There was a problem hiding this comment.
Before
One query that takes ~80ms:
Before raw query and EXPLAIN ANALYZE
EXPLAIN ANALYZE WITH RECURSIVE state(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, state s
WHERE s.state_group = e.state_group
)
SELECT DISTINCT ON (type, state_key)
type, state_key, event_id
FROM state_groups_state
WHERE
state_group IN (
SELECT state_group FROM state
)
AND ((type = 'm.room.history_visibility' AND state_key = '') OR (type = 'm.room.member' AND state_key = '@madlittlemods:matrix.org'))
ORDER BY type, state_key, state_group DESC;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Unique (cost=5274.51..5274.52 rows=1 width=87) (actual time=76.555..76.561 rows=2 loops=1)
CTE state
-> Recursive Union (cost=0.00..284.28 rows=101 width=8) (actual time=0.002..0.443 rows=59 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.57..28.23 rows=10 width=8) (actual time=0.006..0.007 rows=1 loops=59)
-> WorkTable Scan on state s (cost=0.00..0.20 rows=10 width=8) (actual time=0.000..0.000 rows=1 loops=59)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.57..2.79 rows=1 width=16) (actual time=0.006..0.006 rows=1 loops=59)
Index Cond: (state_group = s.state_group)
Heap Fetches: 58
-> Sort (cost=4990.23..4990.23 rows=1 width=87) (actual time=76.554..76.555 rows=2 loops=1)
Sort Key: state_groups_state.type, state_groups_state.state_key, state_groups_state.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=3.11..4990.22 rows=1 width=87) (actual time=0.740..76.542 rows=2 loops=1)
-> HashAggregate (cost=2.27..3.28 rows=101 width=8) (actual time=0.484..0.498 rows=59 loops=1)
Group Key: state.state_group
Batches: 1 Memory Usage: 24kB
-> CTE Scan on state (cost=0.00..2.02 rows=101 width=8) (actual time=0.004..0.459 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state (cost=0.84..49.37 rows=1 width=87) (actual time=0.009..1.289 rows=0 loops=59)
Index Cond: (state_group = state.state_group)
Filter: (((type = 'm.room.history_visibility'::text) AND (state_key = ''::text)) OR ((type = 'm.room.member'::text) AND (state_key = '@madlittlemods:matrix.org'::text)))
Rows Removed by Filter: 1495
Planning Time: 3.309 ms
Execution Time: 76.629 ms
(23 rows)
Time: 80.624 msAfter
Two queries that each take about ~4ms
After raw query (for membership) and EXPLAIN ANALYZE
EXPLAIN ANALYZE WITH RECURSIVE sgs(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, sgs s
WHERE s.state_group = e.state_group
)
SELECT
state_group, type, state_key, event_id
FROM state_groups_state
WHERE
state_group IN (
SELECT state_group FROM sgs
)
AND (type = 'm.room.member' AND state_key = '@madlittlemods:matrix.org')
ORDER BY
type,
state_key,
-- Use the lastest state in the chain
state_group DESC
LIMIT 1;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Limit (cost=779.70..779.70 rows=1 width=87) (actual time=1.248..1.251 rows=1 loops=1)
CTE sgs
-> Recursive Union (cost=0.00..284.28 rows=101 width=8) (actual time=0.003..0.653 rows=59 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.57..28.23 rows=10 width=8) (actual time=0.009..0.010 rows=1 loops=59)
-> WorkTable Scan on sgs s (cost=0.00..0.20 rows=10 width=8) (actual time=0.000..0.000 rows=1 loops=59)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.57..2.79 rows=1 width=16) (actual time=0.008..0.009 rows=1 loops=59)
Index Cond: (state_group = s.state_group)
Heap Fetches: 58
-> Sort (cost=495.42..495.42 rows=1 width=87) (actual time=1.246..1.247 rows=1 loops=1)
Sort Key: state_groups_state.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=3.11..495.41 rows=1 width=87) (actual time=0.925..1.239 rows=1 loops=1)
-> HashAggregate (cost=2.27..3.28 rows=101 width=8) (actual time=0.717..0.729 rows=59 loops=1)
Group Key: sgs.state_group
Batches: 1 Memory Usage: 24kB
-> CTE Scan on sgs (cost=0.00..2.02 rows=101 width=8) (actual time=0.005..0.680 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state (cost=0.84..4.86 rows=1 width=87) (actual time=0.008..0.008 rows=0 loops=59)
Index Cond: ((state_group = sgs.state_group) AND (type = 'm.room.member'::text) AND (state_key = '@madlittlemods:matrix.org'::text))
Planning Time: 2.611 ms
Execution Time: 1.326 ms
(21 rows)
Time: 4.614 ms
After raw query (for history visibility) and EXPLAIN ANALYZE
WITH RECURSIVE sgs(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, sgs s
WHERE s.state_group = e.state_group
)
SELECT
state_group, type, state_key, event_id
FROM state_groups_state
WHERE
state_group IN (
SELECT state_group FROM sgs
)
AND (type = 'm.room.history_visibility' AND state_key = '')
ORDER BY
type,
state_key,
-- Use the lastest state in the chain
state_group DESC
LIMIT 1;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Limit (cost=779.70..779.70 rows=1 width=87) (actual time=0.961..0.964 rows=1 loops=1)
CTE sgs
-> Recursive Union (cost=0.00..284.28 rows=101 width=8) (actual time=0.003..0.458 rows=59 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.002..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.57..28.23 rows=10 width=8) (actual time=0.007..0.007 rows=1 loops=59)
-> WorkTable Scan on sgs s (cost=0.00..0.20 rows=10 width=8) (actual time=0.000..0.000 rows=1 loops=59)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.57..2.79 rows=1 width=16) (actual time=0.006..0.006 rows=1 loops=59)
Index Cond: (state_group = s.state_group)
Heap Fetches: 58
-> Sort (cost=495.42..495.42 rows=1 width=87) (actual time=0.960..0.961 rows=1 loops=1)
Sort Key: state_groups_state.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=3.11..495.41 rows=1 width=87) (actual time=0.675..0.953 rows=1 loops=1)
-> HashAggregate (cost=2.27..3.28 rows=101 width=8) (actual time=0.503..0.514 rows=59 loops=1)
Group Key: sgs.state_group
Batches: 1 Memory Usage: 24kB
-> CTE Scan on sgs (cost=0.00..2.02 rows=101 width=8) (actual time=0.004..0.476 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state (cost=0.84..4.86 rows=1 width=87) (actual time=0.007..0.007 rows=0 loops=59)
Index Cond: ((state_group = sgs.state_group) AND (type = 'm.room.history_visibility'::text) AND (state_key = ''::text))
Planning Time: 1.648 ms
Execution Time: 1.018 ms
(21 rows)
Time: 3.280 ms
There was a problem hiding this comment.
I don't have exact details to be confident in why this is faster but my thinking was we will be trying to do less by matching quicker because of one criteria at a time and LIMIT 1 where it can bail early after one successful state_groups_state match. And not doing any DISTINCT weirdness that probably keeps track of values.
Plus the query planner shows us that we're not throwing away 88k of rows filtering them away anymore.
We still recurse the entire state_event_edges chain but that is probably quick since there are no joins in it.
There was a problem hiding this comment.
Oooh, that is very interesting. I think what is going on here is that the OR condition means that the query planner is no longer just pulling out the specific type/state_key pairs it needs, its instead pulling out the entire state group and filtering in memory. This has significant impact for rooms with large state groups, obviously.
One alternate proposal is instead of using OR conditions is to instead do UNION, e.g.:
EXPLAIN ANALYZE WITH RECURSIVE sgs(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, sgs s
WHERE s.state_group = e.state_group
)
(
SELECT DISTINCT ON (type, state_key)
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.member' AND state_key = '@madlittlemods:matrix.org')
ORDER BY type, state_key, state_group DESC
) UNION (
SELECT DISTINCT ON (type, state_key)
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.history_visibility' AND state_key = '')
ORDER BY type, state_key, state_group DESC
);This has the advantage of being a single query, and reusing the work for pulling out the state edges.
There was a problem hiding this comment.
I think we can actually _get_state_groups_from_groups_txn implementation to do the above if we get a state filter? Would likely be a much smaller change?
There was a problem hiding this comment.
@erikjohnston Have you tested your query? It does seem like an improvement (~50ms) over develop but takes longer than the proposed above and about ~20ms after Postgres cache is warm still.
https://explain.depesz.com/s/cyaR
Raw query plan
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
HashAggregate (cost=1246.10..1246.61 rows=51 width=104) (actual time=28.398..28.403 rows=0 loops=1)
Group Key: state_groups_state.state_group, state_groups_state.type, state_groups_state.state_key, state_groups_state.event_id
Batches: 1 Memory Usage: 24kB
CTE sgs
-> Recursive Union (cost=0.00..515.03 rows=101 width=8) (actual time=0.003..1.594 rows=1 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.28..51.30 rows=10 width=8) (actual time=1.590..1.590 rows=0 loops=1)
-> WorkTable Scan on sgs s (cost=0.00..0.20 rows=10 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.28..5.10 rows=1 width=16) (actual time=1.585..1.585 rows=0 loops=1)
Index Cond: (state_group = s.state_group)
Heap Fetches: 0
-> Append (cost=0.28..730.56 rows=51 width=104) (actual time=28.392..28.395 rows=0 loops=1)
-> Unique (cost=0.28..304.38 rows=1 width=95) (actual time=18.021..18.021 rows=0 loops=1)
-> Nested Loop (cost=0.28..304.38 rows=1 width=95) (actual time=18.020..18.020 rows=0 loops=1)
Join Filter: (state_groups_state.state_group = sgs.state_group)
-> Index Scan Backward using state_groups_state_type_idx on state_groups_state (cost=0.28..301.10 rows=1 width=95) (actual time=18.019..18.019 rows=0 loops=1)
Index Cond: ((type = 'm.room.member'::text) AND (state_key = '@madlittlemods:matrix.org'::text))
-> CTE Scan on sgs (cost=0.00..2.02 rows=101 width=8) (never executed)
-> Unique (cost=425.28..425.41 rows=50 width=95) (actual time=10.369..10.371 rows=0 loops=1)
-> Sort (cost=425.28..425.41 rows=50 width=95) (actual time=10.368..10.370 rows=0 loops=1)
Sort Key: state_groups_state_1.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Hash Join (cost=300.39..423.87 rows=50 width=95) (actual time=9.244..9.245 rows=0 loops=1)
Hash Cond: (state_groups_state_1.state_group = sgs_1.state_group)
-> Bitmap Heap Scan on state_groups_state state_groups_state_1 (cost=297.11..419.88 rows=56 width=95) (actual time=0.921..5.960 rows=232 loops=1)
Recheck Cond: ((type = 'm.room.history_visibility'::text) AND (state_key = ''::text))
Heap Blocks: exact=62
-> Bitmap Index Scan on state_groups_state_type_idx (cost=0.00..297.09 rows=56 width=0) (actual time=0.904..0.905 rows=232 loops=1)
Index Cond: ((type = 'm.room.history_visibility'::text) AND (state_key = ''::text))
-> Hash (cost=2.02..2.02 rows=101 width=8) (actual time=2.140..2.141 rows=1 loops=1)
Buckets: 1024 Batches: 1 Memory Usage: 9kB
-> CTE Scan on sgs sgs_1 (cost=0.00..2.02 rows=101 width=8) (actual time=0.004..1.595 rows=1 loops=1)
Planning Time: 25.952 ms
Execution Time: 33.217 ms
(34 rows)
Time: 141.669 ms
There was a problem hiding this comment.
It takes only a few ms for me :/
There was a problem hiding this comment.
It takes only a few ms for me :/
It's possible I was testing on the wrong database (local vs matrix.org) (kinda hard to judge now that caches are warm eveywhere). We can at least see that it still has a higher cost and deals with more rows.
But I do really like the idea of it being a single query and re-using the state-edge chain!
Talking with @erikjohnston out of band, the plan is to go with the neat UNION solution proposed and I will get a PR for that up in the next few hours (hopefully a much smaller and clean diff for easy review).
Other exploration
Adding LIMIT 1 doesn't seem different and actually slightly higher "cost" (whatever that is worth at this small scale)
https://explain.depesz.com/s/1PQG
Query plan from LIMIT 1
EXPLAIN ANALYZE WITH RECURSIVE sgs(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, sgs s
WHERE s.state_group = e.state_group
)
(
SELECT DISTINCT ON (type, state_key)
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.member' AND state_key = '@madlittlemods:matrix.org')
ORDER BY type, state_key, state_group DESC
LIMIT 1
) UNION (
SELECT DISTINCT ON (type, state_key)
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.history_visibility' AND state_key = '')
ORDER BY type, state_key, state_group DESC
LIMIT 1
);
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Unique (cost=1272.63..1272.66 rows=2 width=104) (actual time=1.541..1.550 rows=2 loops=1)
CTE sgs
-> Recursive Union (cost=0.00..284.28 rows=101 width=8) (actual time=0.003..0.563 rows=59 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.57..28.23 rows=10 width=8) (actual time=0.008..0.008 rows=1 loops=59)
-> WorkTable Scan on sgs s (cost=0.00..0.20 rows=10 width=8) (actual time=0.000..0.000 rows=1 loops=59)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.57..2.79 rows=1 width=16) (actual time=0.007..0.007 rows=1 loops=59)
Index Cond: (state_group = s.state_group)
Heap Fetches: 58
-> Sort (cost=988.35..988.36 rows=2 width=104) (actual time=1.540..1.543 rows=2 loops=1)
Sort Key: state_groups_state.state_group, state_groups_state.type, state_groups_state.state_key, state_groups_state.event_id
Sort Method: quicksort Memory: 25kB
-> Append (cost=494.15..988.34 rows=2 width=104) (actual time=1.159..1.534 rows=2 loops=1)
-> Limit (cost=494.15..494.16 rows=1 width=87) (actual time=1.158..1.160 rows=1 loops=1)
-> Unique (cost=494.15..494.16 rows=1 width=87) (actual time=1.157..1.158 rows=1 loops=1)
-> Sort (cost=494.15..494.16 rows=1 width=87) (actual time=1.156..1.157 rows=1 loops=1)
Sort Key: state_groups_state.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=0.84..494.14 rows=1 width=87) (actual time=1.140..1.151 rows=1 loops=1)
-> CTE Scan on sgs (cost=0.00..2.02 rows=101 width=8) (actual time=0.005..0.591 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state (cost=0.84..4.86 rows=1 width=87) (actual time=0.009..0.009 rows=0 loops=59)
Index Cond: ((state_group = sgs.state_group) AND (type = 'm.room.member'::text) AND (state_key = '@madlittlemods:matrix.org'::text))
-> Limit (cost=494.15..494.16 rows=1 width=87) (actual time=0.370..0.371 rows=1 loops=1)
-> Unique (cost=494.15..494.16 rows=1 width=87) (actual time=0.370..0.370 rows=1 loops=1)
-> Sort (cost=494.15..494.16 rows=1 width=87) (actual time=0.369..0.370 rows=1 loops=1)
Sort Key: state_groups_state_1.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=0.84..494.14 rows=1 width=87) (actual time=0.364..0.366 rows=1 loops=1)
-> CTE Scan on sgs sgs_1 (cost=0.00..2.02 rows=101 width=8) (actual time=0.001..0.012 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state state_groups_state_1 (cost=0.84..4.86 rows=1 width=87) (actual time=0.006..0.006 rows=0 loops=59)
Index Cond: ((state_group = sgs_1.state_group) AND (type = 'm.room.history_visibility'::text) AND (state_key = ''::text))
Planning Time: 3.998 ms
Execution Time: 1.640 ms
(33 rows)
Time: 6.662 ms
Getting rid of the DISTINCT at this point seems to make no difference either:
https://explain.depesz.com/s/eEW
Query plan from LIMIT 1 and no DISTINCT
EXPLAIN ANALYZE WITH RECURSIVE sgs(state_group) AS (
VALUES(739988088::bigint)
UNION ALL
SELECT prev_state_group FROM state_group_edges e, sgs s
WHERE s.state_group = e.state_group
)
(
SELECT
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.member' AND state_key = '@madlittlemods:matrix.org')
ORDER BY type, state_key, state_group DESC
LIMIT 1
) UNION (
SELECT
state_group, type, state_key, event_id
FROM state_groups_state
INNER JOIN sgs USING (state_group)
WHERE (type = 'm.room.history_visibility' AND state_key = '')
ORDER BY type, state_key, state_group DESC
LIMIT 1
);
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Unique (cost=1272.63..1272.66 rows=2 width=104) (actual time=1.537..1.546 rows=2 loops=1)
CTE sgs
-> Recursive Union (cost=0.00..284.28 rows=101 width=8) (actual time=0.003..0.560 rows=59 loops=1)
-> Result (cost=0.00..0.01 rows=1 width=8) (actual time=0.001..0.002 rows=1 loops=1)
-> Nested Loop (cost=0.57..28.23 rows=10 width=8) (actual time=0.008..0.008 rows=1 loops=59)
-> WorkTable Scan on sgs s (cost=0.00..0.20 rows=10 width=8) (actual time=0.000..0.000 rows=1 loops=59)
-> Index Only Scan using state_group_edges_unique_idx on state_group_edges e (cost=0.57..2.79 rows=1 width=16) (actual time=0.007..0.007 rows=1 loops=59)
Index Cond: (state_group = s.state_group)
Heap Fetches: 58
-> Sort (cost=988.35..988.36 rows=2 width=104) (actual time=1.536..1.539 rows=2 loops=1)
Sort Key: state_groups_state.state_group, state_groups_state.type, state_groups_state.state_key, state_groups_state.event_id
Sort Method: quicksort Memory: 25kB
-> Append (cost=494.15..988.34 rows=2 width=104) (actual time=1.144..1.529 rows=2 loops=1)
-> Limit (cost=494.15..494.16 rows=1 width=87) (actual time=1.143..1.145 rows=1 loops=1)
-> Sort (cost=494.15..494.16 rows=1 width=87) (actual time=1.142..1.143 rows=1 loops=1)
Sort Key: state_groups_state.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=0.84..494.14 rows=1 width=87) (actual time=1.125..1.136 rows=1 loops=1)
-> CTE Scan on sgs (cost=0.00..2.02 rows=101 width=8) (actual time=0.005..0.592 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state (cost=0.84..4.86 rows=1 width=87) (actual time=0.009..0.009 rows=0 loops=59)
Index Cond: ((state_group = sgs.state_group) AND (type = 'm.room.member'::text) AND (state_key = '@madlittlemods:matrix.org'::text))
-> Limit (cost=494.15..494.16 rows=1 width=87) (actual time=0.380..0.381 rows=1 loops=1)
-> Sort (cost=494.15..494.16 rows=1 width=87) (actual time=0.380..0.380 rows=1 loops=1)
Sort Key: state_groups_state_1.state_group DESC
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=0.84..494.14 rows=1 width=87) (actual time=0.375..0.377 rows=1 loops=1)
-> CTE Scan on sgs sgs_1 (cost=0.00..2.02 rows=101 width=8) (actual time=0.000..0.013 rows=59 loops=1)
-> Index Scan using state_groups_state_type_idx on state_groups_state state_groups_state_1 (cost=0.84..4.86 rows=1 width=87) (actual time=0.006..0.006 rows=0 loops=59)
Index Cond: ((state_group = sgs_1.state_group) AND (type = 'm.room.history_visibility'::text) AND (state_key = ''::text))
Planning Time: 4.449 ms
Execution Time: 1.632 ms
(31 rows)
Time: 7.024 ms
There was a problem hiding this comment.
Done in v2 -> #14527
MadLittleMods
commented
Nov 22, 2022
MadLittleMods
commented
Nov 22, 2022
Comment on lines
+348
to
+356
| txn.execute(sql, (group, EventTypes.RoomHistoryVisibility, "")) | ||
| history_vis_info = txn.fetchone() | ||
| if history_vis_info is not None: | ||
| row_info_list.append(history_vis_info) | ||
|
|
||
| txn.execute(sql, (group, EventTypes.Member, user_id_viewing_events)) | ||
| membership_info = txn.fetchone() | ||
| if membership_info is not None: | ||
| row_info_list.append(membership_info) |
There was a problem hiding this comment.
Is there a way we can batch up these two individual queries to have less database round-trip time?
Is there a way we can batch up all the queries across all of the state groups?
There was a problem hiding this comment.
Yes, see the UNION idea from @erikjohnston in #14494 (comment)
MadLittleMods
changed the title
filter_events_for_clientfilter_events_for_client for faster /messages
MadLittleMods
commented
Nov 22, 2022
Comment on lines
173
to
+175
| @cancellable | ||
| @trace | ||
| @tag_args |
There was a problem hiding this comment.
Should @cancellable be the innermost or outermost decorator?
I am guessing outermost but perhaps it doesn't matter
MadLittleMods
commented
Nov 22, 2022
Comment on lines
+207
to
+208
| # Since we're making decisions based on the state, we need to wait. | ||
| await_full_state = True |
There was a problem hiding this comment.
Is this accurate?
There was a problem hiding this comment.
Probably the source of our TestPartialStateJoin failures in Complement (https://github.com/matrix-org/synapse/actions/runs/3520947916/jobs/5902365185#step:6:11950). What should I be doing here?
MadLittleMods
commented
Nov 22, 2022
Comment on lines
+113
to
+118
| # Grab the history visibility and membership for each of the events. That's all we | ||
| # need to know in order to filter them. | ||
| event_id_to_state = await storage.state._get_state_for_events_when_filtering_for_client( | ||
| # we exclude outliers at this point, and then handle them separately later | ||
| event_ids=non_outlier_event_ids, | ||
| user_id_viewing_events=user_id, |
There was a problem hiding this comment.
Since we've changed to more individual queries, it's hard to just test all queries that would make up a real life transaction against the database and be 100% confident that everything is better. Can we test this against matrix.org somehow to verify it's an improvement before merging (deploy to a special client_reader worker that you can route my traffic to)?
MadLittleMods
commented
Nov 22, 2022
| event_id_to_state = await storage.state._get_state_for_events_when_filtering_for_client( | ||
| # we exclude outliers at this point, and then handle them separately later | ||
| event_ids=non_outlier_event_ids, | ||
| user_id_viewing_events=user_id, |
There was a problem hiding this comment.
Everything is slightly rushed here and I wish I could have explored more before putting this up for review but this does seem like an improvement and we need to get any change deployed today so I've marked as ready for review.
Let me know if this just isn't right to ship.
4 tasks
MadLittleMods
changed the title
filter_events_for_client for faster /messagesfilter_events_for_client for faster /messages - v1
|
Closing in favor of v2, #14527 |
4 tasks
MadLittleMods
added
the
A-Database
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimize
filter_events_for_client. See #14494 (comment) for SQL query optimization talk. Seems like it will make it a magnitude faster.Fix #14108
Part of #13356 (comment)
Dev notes
Previous optimizations:
Grabbing all of the state via
get_state_for_eventsis the longest part offilter_events_for_clientfilter_events_for_clientget_state_for_eventsget_state_group_for_events_get_state_group_for_events_get_state_for_groups_get_state_for_groups_using_cache_get_state_groups_from_groups🐢_get_state_groups_from_groups_txn(db._get_state_groups_from_groups)get_eventsOf all the state we grab in
filter_events_for_client, we only ever useEventTypes.RoomHistoryVisibilityandEventTypes.Memberfrom itfilter_events_for_client_check_client_allowed_to_see_event_check_filter_send_to_clientstateget_effective_room_visibility_from_stateEventTypes.RoomHistoryVisibility🎈_check_history_visibilityvisibilityresult from above_check_membershipvisibilityresult from aboveEventTypes.Member🎈Relevant database tables:
state_groupsstate_groups_statestate_group_edgesevent_to_state_groupsDocs:
docs/development/room-dag-concepts.mddocs/usage/administration/state_groups.mdThis thing that is part of
_get_state_groups_from_groups_txnseems suspicious:synapse/synapse/storage/databases/state/bg_updates.py
Lines 104 to 106 in e1b15f2
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.Pull request includes a sign off(run the linters)