Rewrite get push actions queries

[Fizzadar]

Previously two large queries were used to fetch push actions for a user, each with a nested query to the receipts table.

This simplifies things by hoisting out the receipts query and then uses a single simplified query to get the push actions and combines the results to get the same data. Two complex queries replaced with two simpler ones.

Part of #13448

Signed off by Nick @ Beeper (@Fizzadar).

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Comparison

Before/current queries:

-- 1st query:

synapse=> EXPLAIN SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions,
   ep.highlight
 FROM (
   SELECT room_id,
       MAX(stream_ordering) as stream_ordering
   FROM events
   INNER JOIN receipts_linearized USING (room_id, event_id)
   WHERE receipt_type IN ('m.read', 'm.read.private') AND user_id = '@fizzadar:beeper.com'
   GROUP BY room_id
) AS rl,
 event_push_actions AS ep
 WHERE
   ep.room_id = rl.room_id
   AND ep.stream_ordering > rl.stream_ordering
    AND ep.stream_ordering > 452600000
    AND ep.stream_ordering <= 4527000000
   AND ep.user_id = '@fizzadar:beeper.com'
   AND ep.notif = 1
 ORDER BY ep.stream_ordering ASC;


-- 2nd query:

EXPLAIN SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions,
    ep.highlight
FROM event_push_actions AS ep
INNER JOIN events AS e USING (room_id, event_id)
WHERE
    ep.room_id NOT IN (
        SELECT room_id FROM receipts_linearized
        WHERE receipt_type IN ('m.read', 'm.read.private') AND user_id = '@fizzadar:beeper.com'
        GROUP BY room_id
    )
    AND ep.user_id = '@fizzadar:beeper.com'
    AND ep.stream_ordering > 452600000
    AND ep.stream_ordering <= 4527000000
    AND ep.notif = 1
ORDER BY ep.stream_ordering ASC;
                                                                                QUERY PLAN
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=181.27..181.28 rows=1 width=97)
   Sort Key: ep.stream_ordering
   ->  Nested Loop  (cost=1.68..181.26 rows=1 width=97)
         ->  GroupAggregate  (cost=1.25..177.63 rows=1 width=39)
               Group Key: events.room_id
               ->  Nested Loop  (cost=1.25..177.61 rows=1 width=39)
                     ->  Index Scan using nick_receipts_linearized_user_room_type on receipts_linearized  (cost=0.56..53.21 rows=32 width=75)
                           Index Cond: (user_id = '@fizzadar:beeper.com'::text)
                           Filter: (receipt_type = ANY ('{m.read,m.read.private}'::text[]))
                     ->  Index Scan using events_event_id_key on events  (cost=0.70..3.88 rows=1 width=83)
                           Index Cond: (event_id = receipts_linearized.event_id)
                           Filter: (receipts_linearized.room_id = room_id)
         ->  Index Scan using event_push_actions_room_id_user_id on event_push_actions ep  (cost=0.42..3.62 rows=1 width=97)
               Index Cond: ((room_id = events.room_id) AND (user_id = '@fizzadar:beeper.com'::text))
               Filter: ((stream_ordering > 452600000) AND (stream_ordering <= '4527000000'::bigint) AND (notif = 1) AND (stream_ordering > (max(events.stream_ordering))))
(15 rows)

                                                                   QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------
 Nested Loop  (cost=32.55..244.71 rows=1 width=97)
   ->  Index Scan using event_push_actions_u_highlight on event_push_actions ep  (cost=31.85..174.73 rows=18 width=97)
         Index Cond: ((user_id = '@fizzadar:beeper.com'::text) AND (stream_ordering > 452600000) AND (stream_ordering <= '4527000000'::bigint))
         Filter: ((NOT (hashed SubPlan 1)) AND (notif = 1))
         SubPlan 1
           ->  Group  (cost=0.56..31.22 rows=32 width=31)
                 Group Key: receipts_linearized.room_id
                 ->  Index Only Scan using nick_receipts_linearized_user_room_type on receipts_linearized  (cost=0.56..31.14 rows=32 width=31)
                       Index Cond: (user_id = '@fizzadar:beeper.com'::text)
                       Filter: (receipt_type = ANY ('{m.read,m.read.private}'::text[]))
   ->  Index Scan using events_event_id_key on events e  (cost=0.70..3.88 rows=1 width=75)
         Index Cond: (event_id = ep.event_id)
         Filter: (ep.room_id = room_id)
(13 rows)

Then after this change:

-- 1st query:

synapse=> EXPLAIN SELECT room_id, MAX(stream_ordering)
FROM receipts_linearized
INNER JOIN events USING (room_id, event_id)
WHERE receipt_type IN ('m.read', 'm.read.private')
AND user_id = '@fizzadar:beeper.com'
GROUP BY room_id;


-- 2nd query:

EXPLAIN SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, ep.highlight
FROM event_push_actions AS ep
WHERE
    ep.user_id = '@fizzadar:beeper.com'
    AND ep.stream_ordering > 452600000
    AND ep.stream_ordering <= 4527000000
    AND ep.notif = 1
ORDER BY ep.stream_ordering ASC;
                                                            QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------
 GroupAggregate  (cost=1.25..177.63 rows=1 width=39)
   Group Key: receipts_linearized.room_id
   ->  Nested Loop  (cost=1.25..177.61 rows=1 width=39)
         ->  Index Scan using nick_receipts_linearized_user_room_type on receipts_linearized  (cost=0.56..53.21 rows=32 width=75)
               Index Cond: (user_id = '@fizzadar:beeper.com'::text)
               Filter: (receipt_type = ANY ('{m.read,m.read.private}'::text[]))
         ->  Index Scan using events_event_id_key on events  (cost=0.70..3.88 rows=1 width=83)
               Index Cond: (event_id = receipts_linearized.event_id)
               Filter: (receipts_linearized.room_id = room_id)
(9 rows)

                                                                QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------
 Index Scan using event_push_actions_u_highlight on event_push_actions ep  (cost=0.55..143.22 rows=37 width=97)
   Index Cond: ((user_id = '@fizzadar:beeper.com'::text) AND (stream_ordering > 452600000) AND (stream_ordering <= '4527000000'::bigint))
   Filter: (notif = 1)
(3 rows)

[clokep]

Would get_unread_push_actions_for_user_in_range_for_email benefit from these changes too? I think the queries are identical except for columns returned.

[Fizzadar]

Yes! Not sure how I missed that... 34be471

[clokep]

I didn't do a full review, just an observation!

[reivilibre]

Sorry, struggling to follow where the optimisation is coming from on this one. (maybe separate commits would have been easier to keep track of, at least for this time of day there are quite a lot of things happening in one diff!) :).

I can see and read from your description that you've unified two queries into one, but it still seems like 2 calls are being made to that query? (and the query doesn't seem to be cached as far as I can see)

What am I missing? :)

[Fizzadar]

Sorry, struggling to follow where the optimisation is coming from on this one. (maybe separate commits would have been easier to keep track of, at least for this time of day there are quite a lot of things happening in one diff!) :).

Totally agree, I struggled to make this into nice commits. Have re-attempted this here: develop...Fizzadar:synapse:optimise-push-actions-queries-rewrite-history - I think this is better?

I can see and read from your description that you've unified two queries into one, but it still seems like 2 calls are being made to that query? (and the query doesn't seem to be cached as far as I can see)

What am I missing? :)

Still two queries, before it was two large queries with a near-identical nested query on the receipts table. This change hoists the receipt query out and executes it first, then a simplified query to get the push actions. Hopefully the new history above will make more sense!

Can overwrite this history with that one if that's OK!

[reivilibre]

yeah, feel free to overwrite the history

[reivilibre]

Looks plausible, couple of small things. Thanks for taking the time to split the commits out for me :)

[reivilibre]

just to be customary, this name should include the _txn suffix because it takes a Transaction.

[Fizzadar]

55f587c

[reivilibre]

to help with readability, especially since you're adding a condition, I'd be tempted to destructure this tuple and then refer to everything by name:

Suggested change

	for row in push_actions
	for event_id, room_id, stream_ordering, actions, highlight in push_actions

[Fizzadar]

Much cleaner! b7a66e3

[reivilibre]

ditto for destructuring here

[Fizzadar]

b7a66e3