Browse Source

fix(hybridcloud) Separate email delivery tasks by silo (#60622)

While the `email` queue task is generally safe to have shared by workers
on different silos, it would be safer if we didn't. Because
`MessageBuilder` handles most email delivery we can use it to separate
workloads.

I've also removed some shim code that was put in place to help with the
django 1.6 to 1.8 upgrade.

---------

Co-authored-by: Alberto Leal <mail4alberto@gmail.com>
Mark Story 1 year ago
parent
commit
c896731288
3 changed files with 18 additions and 11 deletions
  1. 1 1
      src/sentry/conf/server.py
  2. 11 8
      src/sentry/tasks/email.py
  3. 6 2
      src/sentry/utils/email/message_builder.py

+ 1 - 1
src/sentry/conf/server.py

@@ -805,7 +805,7 @@ CELERY_QUEUES_CONTROL = [
     Queue("app_platform.control", routing_key="app_platform.control", exchange=control_exchange),
     Queue("auth.control", routing_key="auth.control", exchange=control_exchange),
     Queue("cleanup.control", routing_key="cleanup.control", exchange=control_exchange),
-    Queue("email", routing_key="email", exchange=control_exchange),
+    Queue("email.control", routing_key="email.control", exchange=control_exchange),
     Queue("integrations.control", routing_key="integrations.control", exchange=control_exchange),
     Queue("files.delete.control", routing_key="files.delete.control", exchange=control_exchange),
     Queue(

+ 11 - 8
src/sentry/tasks/email.py

@@ -58,15 +58,18 @@ def process_inbound_email(mailfrom: str, group_id: int, payload: str):
     queue="email",
     default_retry_delay=60 * 5,
     max_retries=None,
+    silo_mode=SiloMode.REGION,
 )
 def send_email(message):
-    # HACK(django18) Django 1.8 assumes that message objects have a reply_to attribute
-    # When a message is enqueued by django 1.6 we need to patch that property on
-    # so that the message can be converted to a stdlib one.
-    #
-    # See
-    # https://github.com/django/django/blob/c686dd8e6bb3817bcf04b8f13c025b4d3c3dc6dc/django/core/mail/message.py#L273-L274
-    if not hasattr(message, "reply_to"):
-        message.reply_to = []
+    send_messages([message])
 
+
+@instrumented_task(
+    name="sentry.tasks.email.send_email_control",
+    queue="email.control",
+    default_retry_delay=60 * 5,
+    max_retries=None,
+    silo_mode=SiloMode.CONTROL,
+)
+def send_email_control(message):
     send_messages([message])

+ 6 - 2
src/sentry/utils/email/message_builder.py

@@ -20,6 +20,7 @@ from sentry.models.activity import Activity
 from sentry.models.group import Group
 from sentry.models.groupemailthread import GroupEmailThread
 from sentry.models.project import Project
+from sentry.silo.base import SiloMode
 from sentry.utils import json, metrics
 from sentry.utils.safe import safe_execute
 from sentry.web.helpers import render_to_string
@@ -233,7 +234,7 @@ class MessageBuilder:
         cc: Sequence[str] | None = None,
         bcc: Sequence[str] | None = None,
     ) -> None:
-        from sentry.tasks.email import send_email
+        from sentry.tasks.email import send_email, send_email_control
 
         fmt = options.get("system.logging-format")
         messages = self.get_built_messages(to, cc=cc, bcc=bcc)
@@ -244,7 +245,10 @@ class MessageBuilder:
 
         log_mail_queued = partial(logger.info, "mail.queued", extra=extra)
         for message in messages:
-            safe_execute(send_email.delay, message=message, _with_transaction=False)
+            send_email_task = send_email.delay
+            if SiloMode.get_current_mode() == SiloMode.CONTROL:
+                send_email_task = send_email_control.delay
+            safe_execute(send_email_task, message=message, _with_transaction=False)
             extra["message_id"] = message.extra_headers["Message-Id"]
             metrics.incr("email.queued", instance=self.type, skip_internal=False)
             if fmt == LoggingFormat.HUMAN: