From c5c5a7403bd1cb3c9e3a3529d220cb1ea55e89df Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 May 2016 17:01:57 +0100 Subject: [PATCH 1/6] Make synctl read a cache factor from config file --- synapse/app/synctl.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/app/synctl.py b/synapse/app/synctl.py index ab3a31d7b7..669a3ae92a 100755 --- a/synapse/app/synctl.py +++ b/synapse/app/synctl.py @@ -66,6 +66,10 @@ def main(): config = yaml.load(open(configfile)) pidfile = config["pid_file"] + cache_factor = config.get("synctl_cache_factor", None) + + if cache_factor: + os.environ["SYNAPSE_CACHE_FACTOR"] = cache_factor action = sys.argv[1] if sys.argv[1:] else "usage" if action == "start": From c39f305067bf2d4527322922014ff682547de103 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 16 May 2016 17:21:30 +0100 Subject: [PATCH 2/6] os.environ requires a string --- synapse/app/synctl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/synctl.py b/synapse/app/synctl.py index 669a3ae92a..39f4bf6e53 100755 --- a/synapse/app/synctl.py +++ b/synapse/app/synctl.py @@ -69,7 +69,7 @@ def main(): cache_factor = config.get("synctl_cache_factor", None) if cache_factor: - os.environ["SYNAPSE_CACHE_FACTOR"] = cache_factor + os.environ["SYNAPSE_CACHE_FACTOR"] = str(cache_factor) action = sys.argv[1] if sys.argv[1:] else "usage" if action == "start": From 647781ca56c5378250605bdd3c1d69816be03e72 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 16 May 2016 18:40:29 +0100 Subject: [PATCH 3/6] Fix emailpusher import Try importing at the root level rather than conditionally importing, as per comment --- synapse/push/pusher.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/synapse/push/pusher.py b/synapse/push/pusher.py index e6c0806415..de9c33b936 100644 --- a/synapse/push/pusher.py +++ b/synapse/push/pusher.py @@ -18,6 +18,17 @@ from httppusher import HttpPusher import logging logger = logging.getLogger(__name__) +# We try importing this if we can (it will fail if we don't +# have the optional email dependencies installed). We don't +# yet have the config to know if we need the email pusher, +# but importing this after daemonizing seems to fail +# (even though a simple test of importing from a daemonized +# process works fine) +try: + from synapse.push.emailpusher import EmailPusher +except: + pass + def create_pusher(hs, pusherdict): logger.info("trying to create_pusher for %r", pusherdict) @@ -28,7 +39,6 @@ def create_pusher(hs, pusherdict): logger.info("email enable notifs: %r", hs.config.email_enable_notifs) if hs.config.email_enable_notifs: - from synapse.push.emailpusher import EmailPusher PUSHER_TYPES["email"] = EmailPusher logger.info("defined email pusher type") From cbd2adc95e793cb232263efd05b025d6098356af Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 16 May 2016 18:58:38 +0100 Subject: [PATCH 4/6] tune email notifs, fix CSS a bit, and add debugging details --- res/templates/mail.css | 5 +++++ res/templates/notif_mail.html | 11 +++++++++++ synapse/push/emailpusher.py | 32 ++++++++++++++++++++++++-------- synapse/push/mailer.py | 7 ++++++- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/res/templates/mail.css b/res/templates/mail.css index b02f509e58..f2b5e84abc 100644 --- a/res/templates/mail.css +++ b/res/templates/mail.css @@ -2,6 +2,11 @@ body { margin: 0px; } +pre, code { + word-break: break-word; + white-space: pre-wrap; +} + #page { font-family: 'Open Sans', Helvetica, Arial, Sans-Serif; font-color: #454545; diff --git a/res/templates/notif_mail.html b/res/templates/notif_mail.html index 4034f3c601..ba65a80291 100644 --- a/res/templates/notif_mail.html +++ b/res/templates/notif_mail.html @@ -30,6 +30,17 @@ {% include 'room.html' with context %} {% endfor %}
+ + Sending at {{ reason.now|format_ts("%c") }} due to activity in room {{ reason.room_name }} because:
+ 1. The most recent notifiable event was received at {{ reason.received_at|format_ts("%c") }} + which is more than {{ "%.1f"|format(reason.delay_before_mail_ms / (60*1000)) }} (delay_before_mail_ms) mins ago.
+ {% if reason.last_sent_ts %} + 2. The last time we sent a mail for this room was {{ reason.last_sent_ts|format_ts("%c") }}, + which is more than {{ "%.1f"|format(reason.throttle_ms / (60*1000)) }} (current throttle_ms) mins ago. + {% else %} + 2. We can't remember the last time we sent a mail for this room. + {% endif %} + Unsubscribe
diff --git a/synapse/push/emailpusher.py b/synapse/push/emailpusher.py index 3a13c7485a..b4b728adc5 100644 --- a/synapse/push/emailpusher.py +++ b/synapse/push/emailpusher.py @@ -26,11 +26,14 @@ logger = logging.getLogger(__name__) # The amount of time we always wait before ever emailing about a notification # (to give the user a chance to respond to other push or notice the window) -DELAY_BEFORE_MAIL_MS = 2 * 60 * 1000 +DELAY_BEFORE_MAIL_MS = 10 * 60 * 1000 -THROTTLE_START_MS = 2 * 60 * 1000 -THROTTLE_MAX_MS = (2 * 60 * 1000) * (2 ** 11) # ~3 days -THROTTLE_MULTIPLIER = 2 +# THROTTLE is the minimum time between mail notifications sent for a given room. +# Each room maintains its own throttle counter, but each new mail notification +# sends the pending notifications for all rooms. +THROTTLE_START_MS = 10 * 60 * 1000 +THROTTLE_MAX_MS = 24 * 60 * 60 * 1000 # (2 * 60 * 1000) * (2 ** 11) # ~3 days +THROTTLE_MULTIPLIER = 6 # 10 mins, 1 hour, 6 hours, 24 hours # If no event triggers a notification for this long after the previous, # the throttle is released. @@ -146,7 +149,18 @@ class EmailPusher(object): # *one* email updating the user on their notifications, # we then consider all previously outstanding notifications # to be delivered. - yield self.send_notification(unprocessed) + + # debugging: + reason = { + 'room_id': push_action['room_id'], + 'now': self.clock.time_msec(), + 'received_at': received_at, + 'delay_before_mail_ms': DELAY_BEFORE_MAIL_MS, + 'last_sent_ts': self.get_room_last_sent_ts(push_action['room_id']), + 'throttle_ms': self.get_room_throttle_ms(push_action['room_id']), + } + + yield self.send_notification(unprocessed, reason) yield self.save_last_stream_ordering_and_success(max([ ea['stream_ordering'] for ea in unprocessed @@ -195,7 +209,8 @@ class EmailPusher(object): """ Determines whether throttling should prevent us from sending an email for the given room - Returns: True if we should send, False if we should not + Returns: The timestamp when we are next allowed to send an email notif + for this room """ last_sent_ts = self.get_room_last_sent_ts(room_id) throttle_ms = self.get_room_throttle_ms(room_id) @@ -244,8 +259,9 @@ class EmailPusher(object): ) @defer.inlineCallbacks - def send_notification(self, push_actions): + def send_notification(self, push_actions, reason): logger.info("Sending notif email for user %r", self.user_id) + yield self.mailer.send_notification_mail( - self.user_id, self.email, push_actions + self.user_id, self.email, push_actions, reason ) diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py index 2fd38a036a..c2c2ca3fa7 100644 --- a/synapse/push/mailer.py +++ b/synapse/push/mailer.py @@ -92,7 +92,7 @@ class Mailer(object): ) @defer.inlineCallbacks - def send_notification_mail(self, user_id, email_address, push_actions): + def send_notification_mail(self, user_id, email_address, push_actions, reason): raw_from = email.utils.parseaddr(self.hs.config.email_notif_from)[1] raw_to = email.utils.parseaddr(email_address)[1] @@ -143,12 +143,17 @@ class Mailer(object): notifs_by_room, state_by_room, notif_events, user_id ) + reason['room_name'] = calculate_room_name( + state_by_room[reason['room_id']], user_id, fallback_to_members=False + ) + template_vars = { "user_display_name": user_display_name, "unsubscribe_link": self.make_unsubscribe_link(), "summary_text": summary_text, "app_name": self.app_name, "rooms": rooms, + "reason": reason, } html_text = self.notif_template_html.render(**template_vars) From e501e9ecb2026afa0b97348c98b4fb5caa9974f4 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 16 May 2016 19:02:22 +0100 Subject: [PATCH 5/6] tweak text --- res/templates/notif_mail.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/res/templates/notif_mail.html b/res/templates/notif_mail.html index ba65a80291..ced62c19ae 100644 --- a/res/templates/notif_mail.html +++ b/res/templates/notif_mail.html @@ -31,8 +31,8 @@ {% endfor %}
- Sending at {{ reason.now|format_ts("%c") }} due to activity in room {{ reason.room_name }} because:
- 1. The most recent notifiable event was received at {{ reason.received_at|format_ts("%c") }} + Sending email at {{ reason.now|format_ts("%c") }} due to activity in room '{{ reason.room_name }}' because:
+ 1. An event was received at {{ reason.received_at|format_ts("%c") }} which is more than {{ "%.1f"|format(reason.delay_before_mail_ms / (60*1000)) }} (delay_before_mail_ms) mins ago.
{% if reason.last_sent_ts %} 2. The last time we sent a mail for this room was {{ reason.last_sent_ts|format_ts("%c") }}, From 526bf8126f4a865cf7d331f81dec5af42b41b01d Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Tue, 17 May 2016 10:20:51 +0100 Subject: [PATCH 6/6] Remove unused get_joined_rooms_for_user --- synapse/handlers/room_member.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 4e3c1c1c96..7e616f44fd 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -397,21 +397,6 @@ class RoomMemberHandler(BaseHandler): if invite: defer.returnValue(UserID.from_string(invite.sender)) - @defer.inlineCallbacks - def get_joined_rooms_for_user(self, user): - """Returns a list of roomids that the user has any of the given - membership states in.""" - - rooms = yield self.store.get_rooms_for_user( - user.to_string(), - ) - - # For some reason the list of events contains duplicates - # TODO(paul): work out why because I really don't think it should - room_ids = set(r.room_id for r in rooms) - - defer.returnValue(room_ids) - @defer.inlineCallbacks def do_3pid_invite( self,