launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/garbo into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #721195 in Launchpad itself: "Remove garbage from POTranslation"
  https://bugs.launchpad.net/launchpad/+bug/721195

For more details, see:
https://code.launchpad.net/~stub/launchpad/garbo/+merge/56749

Now garbo tasks can be run in parallel, the default static timeouts no longer make sense. Calculate the default task timeout at run time.
-- 
https://code.launchpad.net/~stub/launchpad/garbo/+merge/56749
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/garbo into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2011-03-31 20:55:25 +0000
+++ lib/lp/scripts/garbo.py	2011-04-07 12:44:33 +0000
@@ -962,6 +962,9 @@
     continue_on_failure = False # If True, an exception in a tunable loop
                                 # does not cause the script to abort.
 
+    # Default run time of the script in seconds. Override.
+    default_abort_script_time = None
+
     # _maximum_chunk_size is used to override the defined
     # maximum_chunk_size to allow our tests to ensure multiple calls to
     # __call__ are required without creating huge amounts of test data.
@@ -974,15 +977,19 @@
             test_args=test_args)
 
     def add_my_options(self):
+
         self.parser.add_option("-x", "--experimental", dest="experimental",
             default=False, action="store_true",
             help="Run experimental jobs. Normally this is just for staging.")
         self.parser.add_option("--abort-script",
-            dest="abort_script", default=None, action="store", type="float",
-            metavar="SECS", help="Abort script after SECS seconds.")
+            dest="abort_script", default=self.default_abort_script_time,
+            action="store", type="float", metavar="SECS",
+            help="Abort script after SECS seconds [Default %d]."
+            % self.default_abort_script_time)
         self.parser.add_option("--abort-task",
             dest="abort_task", default=None, action="store", type="float",
-            metavar="SECS", help="Abort a task if it runs over SECS seconds.")
+            metavar="SECS", help="Abort a task if it runs over SECS seconds "
+                "[Default (threads * abort_script / tasks)].")
         self.parser.add_option("--threads",
             dest="threads", default=multiprocessing.cpu_count(),
             action="store", type="int", metavar='NUM',
@@ -1001,9 +1008,10 @@
         else:
             tunable_loops = list(self.tunable_loops)
 
-        a_very_long_time = 31536000 # 1 year
-        abort_task = self.options.abort_task or a_very_long_time
+        a_very_long_time = float(31536000) # 1 year
         abort_script = self.options.abort_script or a_very_long_time
+        abort_task = self.options.abort_task or min(
+            abort_script, options.threads * abort_script / len(tunable_loops))
 
         def worker():
             self.logger.debug(
@@ -1091,12 +1099,7 @@
         ]
     experimental_tunable_loops = []
 
-    def add_my_options(self):
-        super(HourlyDatabaseGarbageCollector, self).add_my_options()
-        # By default, abort any tunable loop taking more than 15 minutes.
-        self.parser.set_defaults(abort_task=900)
-        # And abort the script if it takes more than 55 minutes.
-        self.parser.set_defaults(abort_script=55*60)
+    default_abort_script_time = 60 * 60
 
 
 class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
@@ -1118,7 +1121,4 @@
         PersonPruner,
         ]
 
-    def add_my_options(self):
-        super(DailyDatabaseGarbageCollector, self).add_my_options()
-        # Abort script after 24 hours by default.
-        self.parser.set_defaults(abort_script=86400)
+    default_abort_script_time = 60 * 60 * 24