Merge branch 'bwh/fix-remove-old-jobs' into 'master'

Reduce work and log output from lorry-controller-remove-old-jobs

Closes #18

See merge request CodethinkLabs/lorry/lorry-controller!24

2 files changed, 31 insertions, 27 deletions

diff --git a/lorry-controller-remove-old-jobs b/lorry-controller-remove-old-jobs
index a69e505..d6dca38 100755
--- a/lorry-controller-remove-old-jobs
+++ b/lorry-controller-remove-old-jobs
@@ -26,14 +26,16 @@ import cliapp
 
 class JobInfo(object):
 
-    def __init__(self, job_id, exit_code, exit_timestamp):
+    def __init__(self, job_id, exit_code, exit_timestamp, start_timestamp):
         self.job_id = job_id
         self.exit_code = exit_code
         self.exit_timestamp = exit_timestamp
+        self.start_timestamp = start_timestamp
 
     def __repr__(self):
-        return 'JobInfo(%s,%s,%s)' % (
-            self.job_id, self.exit_code, self.exit_timestamp)
+        return 'JobInfo(%s,%s,%s,%s)' % (
+            self.job_id, self.exit_code, self.exit_timestamp,
+            self.start_timestamp)
 
 
 class OldJobRemover(cliapp.Application):
@@ -69,10 +71,24 @@ class OldJobRemover(cliapp.Application):
     def process_args(self, args):
         logging.info('Removing old jobs from Lorry Controller STATEDB')
 
-        job_ids = self.list_jobs()
-        job_infos = self.get_job_infos(job_ids)
-        ids_of_jobs_to_remove = self.select_for_removal(job_infos)
-        self.remove_jobs(ids_of_jobs_to_remove)
+        for job_id in sorted(self.list_jobs()):
+            try:
+                job_info = self.get_job_info(job_id)
+            except urllib.error.HTTPError as e:
+                logging.warning(
+                    'Trouble getting job info for job %s: %s' %
+                    (job_id, str(e)))
+                continue
+
+            # If the start time of this job is less than max-age ago,
+            # then the finish time of this job, and every job with a
+            # higher ID, must be less than max-age ago.  So we can
+            # stop now.
+            if self.was_started_too_recently(job_info):
+                break
+
+            if self.is_old(job_info):
+                self.remove_job(job_info.job_id)
 
     def list_jobs(self):
         data = self.get('/1.0/list-jobs')
@@ -93,17 +109,6 @@ class OldJobRemover(cliapp.Application):
         parts = (scheme, netloc, path, query, fragment)
         return urllib.parse.urlunsplit(parts)
 
-    def get_job_infos(self, job_ids):
-        job_infos = []
-        for job_id in job_ids:
-            try:
-                job_infos.append(self.get_job_info(job_id))
-            except urllib.error.HTTPError as e:
-                logging.warning(
-                    'Trouble getting job info for job %s: %s' %
-                    (job_id, str(e)))
-        return job_infos
-
     def get_job_info(self, job_id):
         data = self.get('/1.0/job/%s' % job_id)
         obj = json.loads(data.decode('utf-8'))
@@ -112,14 +117,12 @@ class OldJobRemover(cliapp.Application):
             exit_timestamp = self.parse_timestamp(obj['job_ended'])
         else:
             exit_timestamp = None
-        return JobInfo(job_id, exit_code, exit_timestamp)
+        start_timestamp = self.parse_timestamp(obj['job_started'])
+        return JobInfo(job_id, exit_code, exit_timestamp, start_timestamp)
 
     def parse_timestamp(self, timestamp):
         return time.mktime(time.strptime(timestamp, '%Y-%m-%d %H:%M:%S UTC'))
 
-    def select_for_removal(self, job_infos):
-        return [job_info for job_info in job_infos if self.is_old(job_info)]
-
     def is_old(self, job_info):
         if job_info.exit_timestamp is None:
             return False
@@ -127,15 +130,16 @@ class OldJobRemover(cliapp.Application):
         age_in_seconds = current_time - job_info.exit_timestamp
         return age_in_seconds >= self.settings['max-age-in-seconds']
 
+    def was_started_too_recently(self, job_info):
+        current_time = self.get_current_time()
+        age_in_seconds = current_time - job_info.start_timestamp
+        return age_in_seconds < self.settings['max-age-in-seconds']
+
     def get_current_time(self):
         if self.settings['debug-now']:
             return self.settings['debug-now']
         return time.time()
 
-    def remove_jobs(self, job_infos):
-        for job_info in job_infos:
-            self.remove_job(job_info.job_id)
-
     def remove_job(self, job_id):
         logging.info('Removing job %s', job_id)
         self.post('/1.0/remove-job', 'job_id=%s' % job_id)
diff --git a/units/lorry-controller-remove-old-jobs.timer b/units/lorry-controller-remove-old-jobs.timer
index 508a43f..186d4f3 100644
--- a/units/lorry-controller-remove-old-jobs.timer
+++ b/units/lorry-controller-remove-old-jobs.timer
@@ -5,4 +5,4 @@ Description=Lorry Controller remove old jobs
 WantedBy=multi-user.target
 
 [Timer]
-OnUnitInactiveSec=60
+OnUnitInactiveSec=1h