Skip to content

Commit

Permalink
Enforce a Celery singleton across cms and lms by using shared module (#…
Browse files Browse the repository at this point in the history
…25840)

This should prevent the issues we've seen recently where cms modules are
imported by the running lms process, resulting in two celery instances
being created and tasks intermittently being registered to the wrong
instance (and therefore effectively lost.)

In commit ab6bf34/PR #25822 we tried to ensure that only one or the
other of the instances was created by adding a startup check.
Unfortunately, there's an external shared library that refers directly
to the lms celery, causing a startup failure in cms, so we had to revert
it. Rather than waiting to fix that library, this commit collapses
the two instances together so that there is only ever one.
  • Loading branch information
timmc-edx authored Dec 10, 2020
1 parent 999b88e commit 0c57a02
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 26 deletions.
5 changes: 3 additions & 2 deletions cms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import kombu.utils
kombu.utils.entrypoints = lambda namespace: iter([])

# This will make sure the app is always imported when
# Django starts so that shared_task will use this app.
# This will make sure the app is always imported when Django starts so
# that shared_task will use this app, and also ensures that the celery
# singleton is always configured for the CMS.
from .celery import APP as CELERY_APP
14 changes: 3 additions & 11 deletions cms/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,12 @@

import os

from celery import Celery

from openedx.core.lib.celery.routers import AlternateEnvironmentRouter

# set the default Django settings module for the 'celery' program.
# Set the default Django settings module for the 'celery' program
# and then instantiate the Celery singleton.
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'cms.envs.production')

APP = Celery('proj')

APP.conf.task_protocol = 1
# Using a string here means the worker will not have to
# pickle the object when using Windows.
APP.config_from_object('django.conf:settings')
APP.autodiscover_tasks()
from openedx.core.lib.celery import APP # pylint: disable=wrong-import-position,unused-import

# Import after autodiscovery has had a chance to connect to the import_module signal
# so celery doesn't miss any apps getting installed.
Expand Down
5 changes: 3 additions & 2 deletions lms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import kombu.utils
kombu.utils.entrypoints = lambda namespace: iter([])

# This will make sure the app is always imported when
# Django starts so that shared_task will use this app.
# This will make sure the app is always imported when Django starts so
# that shared_task will use this app, and also ensures that the celery
# singleton is always configured for the LMS.
from .celery import APP as CELERY_APP
14 changes: 3 additions & 11 deletions lms/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,12 @@

import os

from celery import Celery

from openedx.core.lib.celery.routers import AlternateEnvironmentRouter

# set the default Django settings module for the 'celery' program.
# Set the default Django settings module for the 'celery' program
# and then instantiate the Celery singleton.
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'lms.envs.production')

APP = Celery('proj')

APP.conf.task_protocol = 1
# Using a string here means the worker will not have to
# pickle the object when using Windows.
APP.config_from_object('django.conf:settings')
APP.autodiscover_tasks()
from openedx.core.lib.celery import APP # pylint: disable=wrong-import-position,unused-import


class Router(AlternateEnvironmentRouter):
Expand Down
30 changes: 30 additions & 0 deletions openedx/core/lib/celery/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""Instantiate the singleton Celery instance that is used by either lms or cms.
WARNING: Do not import this module directly!
This module should only be imported by cms.celery and lms.celery, which perform
setup in a particular order before and after Celery is instantiated. Otherwise,
it might be possible for the Celery singleton to be created without variant-
specific configuration.
The module is intended as a way to have a Celery singleton shared between cms
and lms code. Not having a guaranteed singleton means that it is possible for
each of cms and lms to instantiate Celery, and tasks may be nondeterministically
registered to one or the other of the instances and therefore sometimes lost
to the running process. The root ``__init__.py``s of cms and lms both ensure that
this module is loaded when any cms or lms code runs, effectively using the
Python module system as a singleton loader. (This is an incremental improvement
over older code, and there is probably a better mechanism to be had.)
"""

from celery import Celery

# WARNING: Do not refer to this unless you are cms.celery or
# lms.celery. See module docstring!
APP = Celery('proj')

APP.conf.task_protocol = 1
# Using a string here means the worker will not have to
# pickle the object when using Windows.
APP.config_from_object('django.conf:settings')
APP.autodiscover_tasks()

0 comments on commit 0c57a02

Please sign in to comment.