launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:limit-repacks-to-1000 into launchpad:master.

Commit message:
Limit Git Repacks to 1000 requests per job run

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/401718
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:limit-repacks-to-1000 into launchpad:master.

diff --git a/lib/lp/code/scripts/repackgitrepository.py b/lib/lp/code/scripts/repackgitrepository.py
index 25e8990..2967c0d 100644
--- a/lib/lp/code/scripts/repackgitrepository.py
+++ b/lib/lp/code/scripts/repackgitrepository.py
@@ -27,12 +27,16 @@ from lp.services.looptuner import (
 class RepackTunableLoop(TunableLoop):
     tuner_class = LoopTuner
     maximum_chunk_size = 5
+    # we stop requesting repacks once we've reached
+    # 1000 requests in one run
+    targets = 1000
 
     def __init__(self, log, dry_run, abort_time=None):
         super(RepackTunableLoop, self).__init__(log, abort_time)
         self.dry_run = dry_run
         self.start_at = 0
         self.logger = log
+        self.offset = 0
         self.store = IStore(GitRepository)
 
     def findRepackCandidates(self):
@@ -49,7 +53,10 @@ class RepackTunableLoop(TunableLoop):
         return repos
 
     def isDone(self):
-        return self.findRepackCandidates().is_empty()
+        # we stop at maximum 1000 or when we have no repositories
+        # that are valid repack candidates
+        return (self.findRepackCandidates().is_empty() or
+                self.offset >= self.targets)
 
     def __call__(self, chunk_size):
         repackable_repos = list(self.findRepackCandidates()[:chunk_size])
@@ -62,8 +69,9 @@ class RepackTunableLoop(TunableLoop):
                     self.logger.info(
                         'Requesting automatic git repository repack for %s.'
                         % repo.identity)
-                    repo.repackRepository()
                     counter += 1
+                    self.offset = self.offset + counter
+                    repo.repackRepository()
             except CannotRepackRepository as e:
                 self.logger.error(
                     'An error occurred while requesting repository repack %s'
diff --git a/lib/lp/code/scripts/tests/test_repack_git_repositories.py b/lib/lp/code/scripts/tests/test_repack_git_repositories.py
index 59585cd..786db5d 100644
--- a/lib/lp/code/scripts/tests/test_repack_git_repositories.py
+++ b/lib/lp/code/scripts/tests/test_repack_git_repositories.py
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the repack_git_repositories script."""
+import logging
 import threading
 from wsgiref.simple_server import (
     make_server,
@@ -11,6 +12,7 @@ from wsgiref.simple_server import (
 import transaction
 from zope.security.proxy import removeSecurityProxy
 
+from lp.code.scripts.repackgitrepository import RepackTunableLoop
 from lp.services.config import config
 from lp.services.config.fixture import (
     ConfigFixture,
@@ -67,6 +69,7 @@ class TestRequestGitRepack(TestCaseWithFactory):
 
     def setUp(self):
         super(TestRequestGitRepack, self).setUp()
+        self.log = logging.getLogger('repack')
 
     def runScript_no_Turnip(self):
         transaction.commit()
@@ -135,7 +138,7 @@ class TestRequestGitRepack(TestCaseWithFactory):
 
     def test_auto_repack_with_Turnip_multiple_repos(self):
         # Test repack works when 10 repositories
-        # qualifies for a repack
+        # qualify for a repack
         repo = []
         for i in range(10):
             repo.append(self.factory.makeGitRepository())
@@ -152,3 +155,34 @@ class TestRequestGitRepack(TestCaseWithFactory):
             self.assertIsNotNone(repo[i].date_last_repacked)
             self.assertEqual("/repo/%s/repack" % repo[i].getInternalPath(),
                              self.turnip_server.app.contents[i])
+
+    def test_auto_repack_loop_throttle(self):
+        repacker = FooRepackTunableLoop(self.log, None)
+
+        # Test repack works when 10 repositories
+        # qualifies for a repack but throttle at 7
+        repo = []
+        for i in range(10):
+            repo.append(self.factory.makeGitRepository())
+            repo[i] = removeSecurityProxy(repo[i])
+            repo[i].loose_object_count = 7000
+            repo[i].pack_count = 43
+        transaction.commit()
+
+        # Confirm the initial state is sane.
+        self.assertFalse(repacker.isDone())
+
+        self.makeTurnipServer()
+
+        # Run one loop.
+        repacker(5)
+
+        # although we would have 5 more repos that qualify for a repack at
+        # this point we stop at "targets" defined as 7 for this test but 1 000
+        # in reality
+        self.assertTrue(repacker.isDone())
+
+
+class FooRepackTunableLoop(RepackTunableLoop):
+    maximum_chunk_size = 3
+    targets = 7