launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-retry-timeout-uploads into launchpad:master.

Commit message:
Move RegistryUpload to the slow queue

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/402266

RegistryUpload can regularly exceed the 300 seconds for a job in the normal job queue. The fallback to the slow queue doesn't appear to work correctly, so for now, just use the slow queue directly.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-retry-timeout-uploads into launchpad:master.

diff --git a/lib/lp/oci/model/ocirecipebuildjob.py b/lib/lp/oci/model/ocirecipebuildjob.py
index 8f55850..2b35a47 100644
--- a/lib/lp/oci/model/ocirecipebuildjob.py
+++ b/lib/lp/oci/model/ocirecipebuildjob.py
@@ -45,10 +45,7 @@ from lp.oci.interfaces.ociregistryclient import (
     )
 from lp.services.config import config
 from lp.services.database.enumcol import DBEnum
-from lp.services.database.interfaces import (
-    IMasterStore,
-    IStore,
-    )
+from lp.services.database.interfaces import IStore
 from lp.services.database.locking import (
     AdvisoryLockHeld,
     LockType,
@@ -189,6 +186,10 @@ class OCIRegistryUploadJob(OCIRecipeBuildJobDerived):
 
     class_job_type = OCIRecipeBuildJobType.REGISTRY_UPLOAD
 
+    # This is a known slow task that will exceed the timeouts for
+    # the normal job queue, so put it on a queue with longer timeouts
+    task_queue = 'launchpad_job_slow'
+
     class ManifestListUploadError(Exception):
         pass
 
diff --git a/lib/lp/oci/tests/test_ocirecipebuildjob.py b/lib/lp/oci/tests/test_ocirecipebuildjob.py
index 95718bb..9dfb785 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildjob.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildjob.py
@@ -53,10 +53,7 @@ from lp.services.database.locking import (
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.runner import JobRunner
-from lp.services.job.tests import (
-    block_on_job,
-    pop_remote_notifications,
-    )
+from lp.services.job.tests import block_on_job
 from lp.services.statsd.tests import StatsMixin
 from lp.services.webapp import canonical_url
 from lp.services.webhooks.testing import LogsScheduledWebhooks
@@ -71,7 +68,7 @@ from lp.testing.dbuser import (
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import (
-    CeleryJobLayer,
+    CelerySlowJobLayer,
     DatabaseFunctionalLayer,
     LaunchpadZopelessLayer,
     )
@@ -519,7 +516,6 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin,
 
         self.assertContentEqual([], ocibuild.registry_upload_jobs)
         job = OCIRegistryUploadJob.create(ocibuild)
-        client = FakeRegistryClient()
         switch_dbuser(config.IOCIRegistryUploadJobSource.dbuser)
         # Fork so that we can take an advisory lock from a different
         # PostgreSQL session.
@@ -551,8 +547,6 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory, MultiArchRecipeMixin,
                 os.kill(pid, signal.SIGINT)
 
 
-
-
 class TestOCIRegistryUploadJobViaCelery(TestCaseWithFactory,
                                         MultiArchRecipeMixin):
     """Runs OCIRegistryUploadJob via Celery, to make sure the machinery
@@ -563,7 +557,7 @@ class TestOCIRegistryUploadJobViaCelery(TestCaseWithFactory,
     so we should make sure we are not breaking anything in the interaction
     with the job lifecycle via celery.
     """
-    layer = CeleryJobLayer
+    layer = CelerySlowJobLayer
 
     def setUp(self):
         super(TestOCIRegistryUploadJobViaCelery, self).setUp()
@@ -583,4 +577,5 @@ class TestOCIRegistryUploadJobViaCelery(TestCaseWithFactory,
             for build in builds:
                 OCIRegistryUploadJob.create(build)
             transaction.commit()
-        self.assertEqual(0, len(pop_remote_notifications()))
+        messages = [message.as_string() for message in pop_notifications()]
+        self.assertEqual(0, len(messages))
diff --git a/lib/lp/testing/layers.py b/lib/lp/testing/layers.py
index 1060fa9..541ef56 100644
--- a/lib/lp/testing/layers.py
+++ b/lib/lp/testing/layers.py
@@ -1899,6 +1899,24 @@ class CeleryJobLayer(AppServerLayer):
         cls.celery_worker = None
 
 
+class CelerySlowJobLayer(AppServerLayer):
+    """Layer for tests that run jobs via Celery."""
+
+    celery_worker = None
+
+    @classmethod
+    @profiled
+    def setUp(cls):
+        cls.celery_worker = celery_worker('launchpad_job_slow')
+        cls.celery_worker.__enter__()
+
+    @classmethod
+    @profiled
+    def tearDown(cls):
+        cls.celery_worker.__exit__(None, None, None)
+        cls.celery_worker = None
+
+
 class CeleryBzrsyncdJobLayer(AppServerLayer):
     """Layer for tests that run jobs that read from branches via Celery."""