Merge lp:~adeuring/launchpad/bug-1015667-3 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Abel Deuring |
Approved revision: | no longer in the source branch. |
Merged at revision: | 15656 |
Proposed branch: | lp:~adeuring/launchpad/bug-1015667-3 |
Merge into: | lp:launchpad |
Diff against target: |
265 lines (+117/-23) 7 files modified
lib/lp/services/config/schema-lazr.conf (+7/-1) lib/lp/services/job/celeryconfig.py (+5/-1) lib/lp/services/job/celeryjob.py (+32/-14) lib/lp/services/job/runner.py (+1/-1) lib/lp/services/job/tests/test_celery_configuration.py (+2/-1) lib/lp/services/job/tests/test_celeryjob.py (+69/-4) lib/lp/services/job/tests/test_runner.py (+1/-1) |
To merge this branch: | bzr merge lp:~adeuring/launchpad/bug-1015667-3 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+115110@code.launchpad.net |
Commit message
run the Celery task RunMissingReady with ignore_result=True
Description of the change
This branch fixes bug 1015667: Celery is leaving an ever increasing number
of queues behind. Well, at least I hope that the bug will be fixed --
the result queues created by Celery tasks generally do not give any hint
which task created them: The queue names are the task IDs which are by
default strings generated by uuid4(), and the content of the queued
messages is nothing more than a status message and the data returned by
Task.run(). (OK, this is not completely true: Messages for failed tasks
also contain a traceback -- but that does not help much when we have
tasks that tend to run without failures.)
The culprit was the task run_missing_jobs which simply did not had
the flag ignore_result set.
I changed the task from a decorated function into a class derived
from celery.task.Task and defined the class attribute ignore_result.
This also allows also to override the default task ID. If no ID is
specified as a parameter for celery.
value as mentioned above is created in Task.apply() or
celery.
The test that no result messsage queue is created is somewhat paranoid:
Testing that an unwanted side effect of some other functionality does
not occur must ensure that main event actually happens. Otherwise,
the test might simply check: "If nothing happens, there is no result
queue."
test_run_
list_queued() -- but since this function uses another function
drain_queues() which can also consume queued messages I wanted to be
sure that this does not happen, hence list_queues() is called twice
before celeryd is started.
Side note, for extra fun: have a look at the implementation of
list_queued() and drain_queues() in lazr.jobrunner. drain_queues()
has the parameter "retain"; if it is True, messages are consumed;
list_queued() calls drain_queued(
sane. But the usage of this parameter in drain_queues() scares me:
So no_ack must be False in order to kepp the message in the queue.
And if you want to remove the message, you should set the "don't
acknowledge" flag to False...
This test (and several others for the lazr.jobrunner package and for
lp.servcices.job) could be more simple and more robust if there would
be a way to easily check which queues exist on a rabbitmq instance
and how many messages they contain. There is "rabbitmqctl list_queues"
but this requires root privileges. Also, rabbitmq can provide a
webservice API, but last time I checked it was not available on precise.
And a final rant: After quickly glancing through the AMQP specs I believe
that the protocol does not provide a mecahnism to answer the question
"which queues exist on a given exchange". Scary...
Anyway, I also noticed a minor flaw in BaseRunnableJob
method creates task IDs for regular job runner tasks, similar to
RUnMissingReady
DB ID of a job and separated the different parts (class name, DB ID,
UUID) with '-'. But the '-' are removed from the queue name, so it is
a bit hard to separate the DB ID from the UUID in the queue name.
So I replaced the "separator symbol" '-' with '_'.
tests:
./bin/test services.job -vvt lp.services.
./bin/test services.job -vvt lp.services.
no lint
This branch looks great.