Merge lp:~abentley/launchpad/short-celrerybeat-transactions into lp:launchpad

Proposed by Aaron Bentley
Status: Merged
Approved by: j.c.sackett
Approved revision: no longer in the source branch.
Merged at revision: 15310
Proposed branch: lp:~abentley/launchpad/short-celrerybeat-transactions
Merge into: lp:launchpad
Diff against target: 98 lines (+27/-14)
3 files modified
database/schema/security.cfg (+7/-0)
lib/lp/services/job/celeryjob.py (+12/-12)
lib/lp/services/job/tests/test_celeryjob.py (+8/-2)
To merge this branch: bzr merge lp:~abentley/launchpad/short-celrerybeat-transactions
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+107277@code.launchpad.net

Commit message

run_missing_ready closes transactions and uses its own user

Description of the change

= Summary =
Fix bug #1003720: celerybeat keeps transactions open forever

== Proposed fix ==
Commit transactions and use a unique dbuser

== Pre-implementation notes ==
None

== LOC Rationale ==
I have a LOC credit of 1964.

== Implementation details ==
Can't think of any

== Tests ==
bin/test test_celeryjob

== Demo and Q/A ==
Enable celerybeat. After it has run run_missing_ready once, ask webops whether any transactions are held.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/job/celeryjob.py
  database/schema/security.cfg
  lib/lp/services/job/tests/test_celeryjob.py

To post a comment you must log in.
j.c.sackett (jcsackett) wrote :

Looks good, Aaron. Thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2012-05-24 05:43:49 +0000
3+++ database/schema/security.cfg 2012-05-24 20:10:35 +0000
4@@ -1948,6 +1948,13 @@
5 public.job = SELECT, UPDATE
6 type=user
7
8+[run_missing_ready]
9+groups=script
10+public.branchjob = SELECT
11+public.job = SELECT
12+public.featureflag = SELECT
13+type=user
14+
15 [updateremoteproduct]
16 groups=script
17 public.accesspolicy = SELECT, INSERT
18
19=== modified file 'lib/lp/services/job/celeryjob.py'
20--- lib/lp/services/job/celeryjob.py 2012-05-14 20:41:20 +0000
21+++ lib/lp/services/job/celeryjob.py 2012-05-24 20:10:35 +0000
22@@ -25,10 +25,8 @@
23 from zope.component import getUtility
24
25 from lp.code.model.branchjob import BranchScanJob
26-from lp.services.config import (
27- config,
28- dbconfig,
29- )
30+from lp.scripts.helpers import TransactionFreeOperation
31+from lp.services.config import dbconfig
32 from lp.services.database.lpstorm import IStore
33 from lp.services.features import (
34 install_feature_controller,
35@@ -89,14 +87,16 @@
36 :param _no_init: For tests. If True, do not perform the initialization.
37 """
38 if not _no_init:
39- task_init(config.launchpad.dbuser)
40- count = 0
41- for job in find_missing_ready(BranchScanJob):
42- if not celery_enabled(job.__class__.__name__):
43- continue
44- job.celeryCommitHook(True)
45- count += 1
46- info('Scheduled %d missing jobs.', count)
47+ task_init('run_missing_ready')
48+ with TransactionFreeOperation():
49+ count = 0
50+ for job in find_missing_ready(BranchScanJob):
51+ if not celery_enabled(job.__class__.__name__):
52+ continue
53+ job.celeryCommitHook(True)
54+ count += 1
55+ info('Scheduled %d missing jobs.', count)
56+ transaction.commit()
57
58
59 needs_zcml = True
60
61=== modified file 'lib/lp/services/job/tests/test_celeryjob.py'
62--- lib/lp/services/job/tests/test_celeryjob.py 2012-05-14 20:06:58 +0000
63+++ lib/lp/services/job/tests/test_celeryjob.py 2012-05-24 20:10:35 +0000
64@@ -2,12 +2,14 @@
65 # GNU Affero General Public License version 3 (see the file LICENSE).
66
67 from lp.code.model.branchjob import BranchScanJob
68+from lp.scripts.helpers import TransactionFreeOperation
69 from lp.services.features.testing import FeatureFixture
70 from lp.services.job.tests import (
71 drain_celery_queues,
72 monitor_celery,
73 )
74 from lp.testing import TestCaseWithFactory
75+from lp.testing.dbuser import dbuser
76 from lp.testing.layers import ZopelessAppServerLayer
77
78
79@@ -42,7 +44,9 @@
80 """run_missing_ready does nothing if the class isn't enabled."""
81 self.createMissingJob()
82 with monitor_celery() as responses:
83- self.run_missing_ready(_no_init=True)
84+ with dbuser('run_missing_ready'):
85+ with TransactionFreeOperation.require():
86+ self.run_missing_ready(_no_init=True)
87 self.assertEqual([], responses)
88
89 def test_run_missing_ready(self):
90@@ -51,5 +55,7 @@
91 self.useFixture(
92 FeatureFixture({'jobs.celery.enabled_classes': 'BranchScanJob'}))
93 with monitor_celery() as responses:
94- self.run_missing_ready(_no_init=True)
95+ with dbuser('run_missing_ready'):
96+ with TransactionFreeOperation.require():
97+ self.run_missing_ready(_no_init=True)
98 self.assertEqual(1, len(responses))