Skip to content

Commit

Permalink
bug 1728210: remove celery tasks
Browse files Browse the repository at this point in the history
This is the first step in removing celery usage. This removes all the
tasks and folds the important bits into the code that kicks off those
tasks.

* for tecken.download, if there's a problem saving the missing symbol to
  the db, we let it go instead of spinning that off into a task
* for tecken.upload, we fold the uploads_created bookkeeping into the
  upload task; prod dashboard shows this takes a few seconds at most and
  usually it's under half a second
* don't need a sample_task or the infrastructure to test that celery is
  working anymore
* for tecken.upload, we nix invalidating the symbolicate cache--this
  code isn't long for this world and socorro doesn't invalidate its
  cache on uploads, to I think it's ok to degrade the service here
  • Loading branch information
willkg committed Sep 10, 2021
1 parent 7b41a9e commit a7e5dc6
Show file tree
Hide file tree
Showing 27 changed files with 51 additions and 561 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ setup: .env ## | Initialize services.

.PHONY: run
run: .env .docker-build ## | Run the web app and services.
docker-compose up web eliot worker frontend
docker-compose up web eliot frontend

.PHONY: stop
stop: .env ## | Stop docker containers.
Expand Down Expand Up @@ -76,8 +76,8 @@ redis-store-cli: .env .docker-build ## | Open Redis CLI to store Redis server.

.PHONY: psql
psql: .env .docker-build ## | Open psql cli.
@echo "Password is 'postgres'."
docker-compose run --rm db psql -h db -U postgres
@echo "NOTE: Password is 'postgres'."
docker-compose run --rm db psql -h db -U postgres -d tecken

.PHONY: test
test: .env .docker-build ## | Run Python unit test suite.
Expand Down
2 changes: 2 additions & 0 deletions bin/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ eliot) ## Run Eliot service
exec honcho -f /app/eliot-service/Procfile start
;;
worker) ## Run Celery worker
# FIXME(willkg): 1728210: remove this after we remove the celery infra
exec ${CMD_PREFIX} celery -A tecken.celery:app worker --loglevel INFO
;;
worker-purge) ## Purge Celery tasks
# FIXME(willkg): 1728210: remove this after we remove the celery infra
# Start worker but first purge ALL old stale tasks.
# Only useful in local development where you might have accidentally
# started waaaay too make background tasks when debugging something.
Expand Down
12 changes: 0 additions & 12 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,6 @@ services:
- redis-cache
command: web

worker:
extends:
service: base
depends_on:
- base
links:
- db
- redis-cache
volumes:
- $PWD:/app
command: worker-purge

frontend:
build:
context: .
Expand Down
45 changes: 0 additions & 45 deletions docs/celery.rst

This file was deleted.

1 change: 0 additions & 1 deletion docs/diagrams/arch.gv
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ digraph G {

webapp [shape=rect, label="webapp"];
eliotwebapp [shape=rect, label="eliot webapp"];
celeryworker [shape=rect, label="celery"];
}

subgraph stores {
Expand Down
34 changes: 0 additions & 34 deletions docs/endtoendtesting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,37 +43,3 @@ it will output something like this:
To see it, go to: https://symbols.stage.mozaws.net/uploads/upload/1186
It worked! 🎉 🎊 👍🏼 🌈
.. _endtoendtesting-celery:

Celery
======

To test that the relationship between the web app and the Celery worker is
worker you can use a special, and public, endpoint called ``/__task_tester__``.
When you send a HTTP POST request to it, it starts a Celery job that
writes to the main cache (Redis). Then, if you do a HTTP GET request
afterwards, it will either respond with 200 OK if the cache got updated
or 500 Internal Server Error if the cache did not get updated.

To run the test, first HTTP POST as per this example...:

.. code-block:: shell
▶ curl --user-agent "e2etesting/1.0" -v -XPOST localhost:8000/__task_tester__
> POST /__task_tester__ HTTP/1.1
>
< HTTP/1.1 201 Created
<
Now make a GET request to this URL
Then, the HTTP GET:

.. code-block:: shell
▶ curl --user-agent "e2etesting/1.0" -v localhost:8000/__task_tester__
> GET /__task_tester__ HTTP/1.1
>
< HTTP/1.1 200 OK
<
It works!
Binary file modified docs/images/arch.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ used by Mozilla.
contributing
dev
configuration
celery
frontend
redis
adr_log
Expand Down
1 change: 1 addition & 0 deletions tecken/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at http://mozilla.org/MPL/2.0/.

# FIXME(willkg): remove after we remove celery infra
from .celery import app as celery_app

default_app_config = "tecken.apps.TeckenAppConfig"
Expand Down
3 changes: 3 additions & 0 deletions tecken/boto_extra.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
from botocore.exceptions import EndpointConnectionError, ClientError


# FIXME(willkg): 1728210: remove this after we remove celery infra


class OwnEndpointConnectionError(EndpointConnectionError):
"""Because the botocore.exceptions.EndpointConnectionError can't be
pickled, if this exception happens during task work, celery
Expand Down
6 changes: 1 addition & 5 deletions tecken/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at http://mozilla.org/MPL/2.0/.

# FIXME(willkg): 1728210: remove this after we remove celery infra
import os

from celery import Celery
Expand All @@ -25,11 +26,6 @@
# should have a `CELERY_` prefix.
app.config_from_object("django.conf:settings", namespace="CELERY")

# Specifically list the apps that have tasks.py
# Note! If not doing this you get a strange RuntimeError
# ('path' must be None or a list, not <class '_frozen_importlib_external._NamespacePath'>) # noqa
app.autodiscover_tasks(["tecken", "tecken.download", "tecken.symbolicate"])


@app.task(bind=True)
def debug_task(self):
Expand Down
22 changes: 0 additions & 22 deletions tecken/download/tasks.py

This file was deleted.

34 changes: 14 additions & 20 deletions tecken/download/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from tecken.base.utils import invalid_key_name_characters
from tecken.download.models import MissingSymbol
from tecken.download.utils import store_missing_symbol
from tecken.download.tasks import store_missing_symbol_task
from tecken.download.forms import DownloadForm, DownloadsMissingForm
from tecken.storage import StorageBucket

Expand Down Expand Up @@ -163,22 +162,19 @@ def download_symbol(request, symbol, debugid, filename, try_symbols=False):
def log_symbol_get_404(symbol, debugid, filename, code_file="", code_id=""):
"""Store the fact that a symbol could not be found.
The purpose of this is be able to answer "What symbol fetches have
recently been attempted and failed?" With that knowledge, we can
deduce which symbols are commonly needed in symbolication but failed
to be available. Then you can try to go and get hold of them and
thus have less symbol 404s in the future.
Because this is expected to be called A LOT (in particular from
Socorro's Processor) we have to do this rapidly in a database
that is suitable for many fast writes.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1361854#c5
for the backstory about expected traffic.
The URL used when requesting the file will only ever be
'symbol', 'debugid' and 'filename', but some services, like Socorro's
stackwalker is actually aware of other parameters that are
relevant only to this URL. Hence 'code_file' and 'code_id' which
The purpose of this is be able to answer "What symbol fetches have recently been
attempted and failed?" With that knowledge, we can deduce which symbols are commonly
needed in symbolication but failed to be available. Then you can try to go and get
hold of them and thus have less symbol 404s in the future.
Because this is expected to be called A LOT (in particular from Socorro's Processor)
we have to do this rapidly in a database that is suitable for many fast writes. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1361854#c5 for the backstory about
expected traffic.
The URL used when requesting the file will only ever be 'symbol', 'debugid' and
'filename', but some services, like Socorro's stackwalker is actually aware of other
parameters that are relevant only to this URL. Hence 'code_file' and 'code_id' which
are both optional.
"""
Expand All @@ -188,9 +184,7 @@ def log_symbol_get_404(symbol, debugid, filename, code_file="", code_id=""):
symbol, debugid, filename, code_file=code_file, code_id=code_id
)
except OperationalError:
store_missing_symbol_task.delay(
symbol, debugid, filename, code_file=code_file, code_id=code_id
)
logger.exception("error when storing missing symbol")


@metrics.timer("missingsymbols_csv")
Expand Down
1 change: 1 addition & 0 deletions tecken/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class AWS:
}


# FIXME(willkg): 1728210: remove after we remove celery infra
class Celery:

# Add a 5 minute soft timeout to all Celery tasks.
Expand Down
12 changes: 0 additions & 12 deletions tecken/symbolicate/tasks.py

This file was deleted.

18 changes: 0 additions & 18 deletions tecken/symbolicate/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import hashlib

from django.core.cache import caches
from django.conf import settings


Expand Down Expand Up @@ -38,20 +37,3 @@ def make_symbol_key_cache_key(symbol_key, prefix=None):
assert isinstance(symbol_key, (tuple, list)), symbol_key
assert len(symbol_key) == 2, symbol_key
return "symbol:{}:{}/{}".format(prefix, *symbol_key)


def invalidate_symbolicate_cache(symbol_keys, prefix=None):
"""Makes sure all symbolication caching stored for this list of
symbol keys is removed from the Redis store."""
all_keys = []
for symbol_key in symbol_keys:
# Every symbol, for the sake of symbolication, is stored in two
# ways:
# 1) symbol_key + ':keys' (plain SET)
# 2) symbol_key (as hashmap)
cache_key = make_symbol_key_cache_key(symbol_key, prefix=prefix)
all_keys.append(cache_key) # the hashmap
all_keys.append(cache_key + ":keys") # the list of all offsets

store = caches["store"]
store.delete_many(all_keys)
23 changes: 0 additions & 23 deletions tecken/tasks.py

This file was deleted.

45 changes: 0 additions & 45 deletions tecken/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,6 @@ def test_something(requestsmock):
yield m


@pytest.fixture
def celery_config():
return {
"broker_url": "redis://redis-cache:6379/0",
"result_backend": "redis://redis-cache:6379/0",
"task_always_eager": True,
}


# This needs to be imported at least once. Otherwise the mocking
# done in botomock() doesn't work.
# (peterbe) Would like to know why but for now let's just comply.
Expand Down Expand Up @@ -170,42 +161,6 @@ def fakeuser():
return User.objects.create(username="peterbe", email="[email protected]")


def _mock_invalidate_symbolicate_cache(function_path):
class FakeTask:
all_delay_arguments = []

def delay(self, *args, **kwargs):
self.all_delay_arguments.append((args, kwargs))

fake_task = FakeTask()

with mock.patch(function_path, new=fake_task):
yield fake_task


@pytest.fixture
def upload_mock_invalidate_symbolicate_cache():
"""Yields an object that is the mocking substitute of some task
functions that are imported by the views.
If a view function (that you know your test will execute) depends
on 'tecken.symbolicate.tasks.invalidate_symbolicate_cache', add
this fixture to your test. Then you can access all the arguments
sent to it as `.delay()` arguments and keyword arguments.
"""

class FakeTask:
all_delay_arguments = []

def delay(self, *args, **kwargs):
self.all_delay_arguments.append((args, kwargs))

fake_task = FakeTask()

_mock_function = "tecken.upload.views.invalidate_symbolicate_cache_task"
with mock.patch(_mock_function, new=fake_task):
yield fake_task


@pytest.fixture
def upload_mock_update_uploads_created_task():
"""Yields an object that is the mocking substitute of some task
Expand Down
Loading

0 comments on commit a7e5dc6

Please sign in to comment.