Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Be more agressive about purging old room event_push_actions #1873

Merged
merged 1 commit into from Feb 14, 2017

Conversation

erikjohnston
Copy link
Member

There's no reason to keep a months worth of notifications after they've been read. While the pushers do use the table, they shouldn't be needing to send anything that has already been read.

We keep a months worth of highlights so that the notification API still works.

@erikjohnston
Copy link
Member Author

Actually, the API does allow you to paginate on plain notifications, though I don't think anybody uses it.

@ara4n
Copy link
Member

ara4n commented Feb 1, 2017 via email

@erikjohnston
Copy link
Member Author

@ara4n They are just highlights I thought?

@@ -475,7 +479,8 @@ def _remove_old_push_actions_before_txn(self, txn, room_id, user_id,
txn.execute(
"DELETE FROM event_push_actions "
" WHERE user_id = ? AND room_id = ? AND "
" topological_ordering < ? AND stream_ordering < ?",
" topological_ordering < ?"
" AND ((stream_ordering < ? AND highlight = 1) or highlight = 0)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalise "OR" please.

@NegativeMjark
Copy link
Contributor

Apart from the capitalisation LGTM.

The query plans before and after are:

 Delete on event_push_actions  (cost=0.70..8.72 rows=1 width=6)
   ->  Index Scan using event_push_actions_rm_tokens on event_push_actions  (cost=0.70..8.72 rows=1 width=6)
         Index Cond: ((user_id = '?'::text) AND (room_id = '?'::text) AND (topological_ordering < ?) AND (stream_ordering < ?))
 Delete on event_push_actions  (cost=0.70..8.73 rows=1 width=6)
   ->  Index Scan using event_push_actions_rm_tokens on event_push_actions  (cost=0.70..8.73 rows=1 width=6)
         Index Cond: ((user_id = '?'::text) AND (room_id = '?'::text) AND (topological_ordering < ?))
         Filter: (((stream_ordering < ?) AND (highlight = 1)) OR (highlight = 0))

I don't think it matters that we're losing the stream_ordering from the index cond, since it'll need to scan for topological_ordering < ? in either case, and this should help performance because more of the stuff matching topological_ordering < ? will be deleted.

@erikjohnston erikjohnston merged commit 795f8e3 into develop Feb 14, 2017
erikjohnston added a commit that referenced this pull request Mar 13, 2017
Changes in synapse v0.19.3-rc1 (2017-03-08)
===========================================

Features:

* Add some administration functionalities. Thanks to morteza-araby! (PR #1784)

Changes:

* Reduce database table sizes (PR #1873, #1916, #1923, #1963)
* Update contrib/ to not use syutil. Thanks to andrewshadura! (PR #1907)
* Don't fetch current state when sending an event in common case (PR #1955)

Bug fixes:

* Fix synapse_port_db failure. Thanks to Pneumaticat! (PR #1904)
* Fix caching to not cache error responses (PR #1913)
* Fix APIs to make kick & ban reasons work (PR #1917)
* Fix bugs in the /keys/changes api (PR #1921)
* Fix bug where users couldn't forget rooms they were banned from (PR #1922)
* Fix issue with long language values in pushers API (PR #1925)
* Fix a race in transaction queue (PR #1930)
* Fix dynamic thumbnailing to preserve aspect ratio. Thanks to jkolo! (PR
  #1945)
* Fix device list update to not constantly resync (PR #1964)
* Fix potential for huge memory usage when getting device that have
  changed (PR #1969)
@erikjohnston erikjohnston deleted the erikj/delete_push_actions branch March 29, 2017 10:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants