From acac21248cf1834233831383ee52198ca1bd010c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 15 Feb 2018 15:01:12 +0000 Subject: Store push actions in staging area --- synapse/storage/event_push_actions.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'synapse/storage/event_push_actions.py') diff --git a/synapse/storage/event_push_actions.py b/synapse/storage/event_push_actions.py index 8efe2fd4bb..80c3cfe95f 100644 --- a/synapse/storage/event_push_actions.py +++ b/synapse/storage/event_push_actions.py @@ -738,6 +738,33 @@ class EventPushActionsStore(SQLBaseStore): (rotate_to_stream_ordering,) ) + def add_push_actions_to_staging(self, event_id, user_id, actions): + """Add the push actions for the user and event to the push + action staging area. + + Args: + event_id (str) + user_id (str) + actions (list) + + Returns: + Deferred + """ + + is_highlight = _action_has_highlight(actions) + + return self._simple_insert( + table="event_push_actions_staging", + values={ + "event_id": event_id, + "user_id": user_id, + "actions": _serialize_action(actions, is_highlight), + "notif": True, + "highlight": is_highlight, + }, + desc="add_push_actions_to_staging", + ) + def _action_has_highlight(actions): for action in actions: -- cgit 1.5.1 From c714c6185367e39123530cb7f89584004434c473 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 15 Feb 2018 15:13:36 +0000 Subject: Update event_push_actions table from staging table --- synapse/storage/event_push_actions.py | 59 ++++++++++++++++++++++------------- synapse/storage/events.py | 2 +- 2 files changed, 39 insertions(+), 22 deletions(-) (limited to 'synapse/storage/event_push_actions.py') diff --git a/synapse/storage/event_push_actions.py b/synapse/storage/event_push_actions.py index 80c3cfe95f..34ff9be731 100644 --- a/synapse/storage/event_push_actions.py +++ b/synapse/storage/event_push_actions.py @@ -88,33 +88,50 @@ class EventPushActionsStore(SQLBaseStore): self._rotate_notifs, 30 * 60 * 1000 ) - def _set_push_actions_for_event_and_users_txn(self, txn, event, tuples): + def _set_push_actions_for_event_and_users_txn(self, txn, event): """ Args: event: the event set actions for tuples: list of tuples of (user_id, actions) """ - values = [] - for uid, actions in tuples: - is_highlight = 1 if _action_has_highlight(actions) else 0 - - values.append({ - 'room_id': event.room_id, - 'event_id': event.event_id, - 'user_id': uid, - 'actions': _serialize_action(actions, is_highlight), - 'stream_ordering': event.internal_metadata.stream_ordering, - 'topological_ordering': event.depth, - 'notif': 1, - 'highlight': is_highlight, - }) - - for uid, __ in tuples: + + sql = """ + INSERT INTO event_push_actions ( + room_id, event_id, user_id, actions, stream_ordering, + topological_ordering, notif, highlight + ) + SELECT ?, event_id, user_id, actions, ?, ?, notif, highlight + FROM event_push_actions_staging + WHERE event_id = ? + """ + + txn.execute(sql, ( + event.room_id, event.internal_metadata.stream_ordering, + event.depth, event.event_id, + )) + + user_ids = self._simple_select_onecol_txn( + txn, + table="event_push_actions_staging", + keyvalues={ + "event_id": event.event_id, + }, + retcol="user_id", + ) + + self._simple_delete_txn( + txn, + table="event_push_actions_staging", + keyvalues={ + "event_id": event.event_id, + }, + ) + + for uid in user_ids: txn.call_after( self.get_unread_event_push_actions_by_room_for_user.invalidate_many, - (event.room_id, uid) + (event.room_id, uid,) ) - self._simple_insert_many_txn(txn, "event_push_actions", values) @cachedInlineCallbacks(num_args=3, tree=True, max_entries=5000) def get_unread_event_push_actions_by_room_for_user( @@ -751,7 +768,7 @@ class EventPushActionsStore(SQLBaseStore): Deferred """ - is_highlight = _action_has_highlight(actions) + is_highlight = is_highlight = 1 if _action_has_highlight(actions) else 0 return self._simple_insert( table="event_push_actions_staging", @@ -759,7 +776,7 @@ class EventPushActionsStore(SQLBaseStore): "event_id": event_id, "user_id": user_id, "actions": _serialize_action(actions, is_highlight), - "notif": True, + "notif": 1, "highlight": is_highlight, }, desc="add_push_actions_to_staging", diff --git a/synapse/storage/events.py b/synapse/storage/events.py index 28cce2979c..ca64aacb1c 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -1170,7 +1170,7 @@ class EventsStore(SQLBaseStore): # Insert all the push actions into the event_push_actions table. if context.push_actions: self._set_push_actions_for_event_and_users_txn( - txn, event, context.push_actions + txn, event, ) if event.type == EventTypes.Redaction and event.redacts is not None: -- cgit 1.5.1 From b96278d6fe499e47133d2d2e82b9d3a0074d7005 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 15 Feb 2018 15:37:40 +0000 Subject: Ensure that we delete staging push actions on errors --- synapse/handlers/message.py | 12 +++++++++--- synapse/storage/event_push_actions.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) (limited to 'synapse/storage/event_push_actions.py') diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 1c3ac03f20..d99d8049b3 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -683,9 +683,15 @@ class EventCreationHandler(object): event, context ) - (event_stream_id, max_stream_id) = yield self.store.persist_event( - event, context=context - ) + try: + (event_stream_id, max_stream_id) = yield self.store.persist_event( + event, context=context + ) + except: # noqa: E722, as we reraise the exception this is fine. + # Ensure that we actually remove the entries in the push actions + # staging area + preserve_fn(self.store.remove_push_actions_from_staging)(event.event_id) + raise # this intentionally does not yield: we don't care about the result # and don't need to wait for it. diff --git a/synapse/storage/event_push_actions.py b/synapse/storage/event_push_actions.py index 34ff9be731..28226455bf 100644 --- a/synapse/storage/event_push_actions.py +++ b/synapse/storage/event_push_actions.py @@ -782,6 +782,22 @@ class EventPushActionsStore(SQLBaseStore): desc="add_push_actions_to_staging", ) + def remove_push_actions_from_staging(self, event_id): + """Called if we failed to persist the event to ensure that stale push + actions don't build up in the DB + + Args: + event_id (str) + """ + + return self._simple_delete( + table="event_push_actions_staging", + keyvalues={ + "event_id": event_id, + }, + desc="remove_push_actions_from_staging", + ) + def _action_has_highlight(actions): for action in actions: -- cgit 1.5.1 From 012e8e142a4ca7d87e1ffd66cce44b23bf943e9c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 16 Feb 2018 11:35:01 +0000 Subject: Comments --- synapse/push/bulk_push_rule_evaluator.py | 3 +++ synapse/storage/event_push_actions.py | 3 ++- synapse/storage/schema/delta/47/push_actions_staging.sql | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) (limited to 'synapse/storage/event_push_actions.py') diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 1140788aa7..bf4f1c5836 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -189,6 +189,9 @@ class BulkPushRuleEvaluator(object): if matches: actions = [x for x in rule['actions'] if x != 'dont_notify'] if actions and 'notify' in actions: + # Push rules say we should notify the user of this event, + # so we mark it in the DB in the staging area. (This + # will then get handled when we persist the event) yield self.store.add_push_actions_to_staging( event.event_id, uid, actions, ) diff --git a/synapse/storage/event_push_actions.py b/synapse/storage/event_push_actions.py index 28226455bf..ea56d4d065 100644 --- a/synapse/storage/event_push_actions.py +++ b/synapse/storage/event_push_actions.py @@ -762,7 +762,8 @@ class EventPushActionsStore(SQLBaseStore): Args: event_id (str) user_id (str) - actions (list) + actions (list[dict|str]): An action can either be a string or + dict. Returns: Deferred diff --git a/synapse/storage/schema/delta/47/push_actions_staging.sql b/synapse/storage/schema/delta/47/push_actions_staging.sql index ec4b1d7d42..edccf4a96f 100644 --- a/synapse/storage/schema/delta/47/push_actions_staging.sql +++ b/synapse/storage/schema/delta/47/push_actions_staging.sql @@ -13,6 +13,10 @@ * limitations under the License. */ +-- Temporary staging area for push actions that have been calculated for an +-- event, but the event hasn't yet been persisted. +-- When the event is persisted the rows are moved over to the +-- event_push_actions table. CREATE TABLE event_push_actions_staging ( event_id TEXT NOT NULL, user_id TEXT NOT NULL, -- cgit 1.5.1 From 6af025d3c4c19ab8a6f90b667b8c4259606ba47a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 16 Feb 2018 11:35:31 +0000 Subject: Fix typo of double is_highlight --- synapse/storage/event_push_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/storage/event_push_actions.py') diff --git a/synapse/storage/event_push_actions.py b/synapse/storage/event_push_actions.py index ea56d4d065..f787431b7a 100644 --- a/synapse/storage/event_push_actions.py +++ b/synapse/storage/event_push_actions.py @@ -769,7 +769,7 @@ class EventPushActionsStore(SQLBaseStore): Deferred """ - is_highlight = is_highlight = 1 if _action_has_highlight(actions) else 0 + is_highlight = 1 if _action_has_highlight(actions) else 0 return self._simple_insert( table="event_push_actions_staging", -- cgit 1.5.1