From be3383e67b115a4980eb1ef47a84bdcf8c5fa028 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Thu, 9 Apr 2015 12:01:01 +0000 Subject: distbuild: Track state of a job in the Job class This is a bit more comprehensive than the previous approach of using public instance attributes, and I find it easier to reason about. Change-Id: I2942ecf53c95e29893dc0982d38aec689ebfa614 --- distbuild/worker_build_scheduler.py | 53 ++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 22 deletions(-) (limited to 'distbuild') diff --git a/distbuild/worker_build_scheduler.py b/distbuild/worker_build_scheduler.py index 4c5bd545..843b9eb9 100644 --- a/distbuild/worker_build_scheduler.py +++ b/distbuild/worker_build_scheduler.py @@ -105,8 +105,26 @@ class Job(object): self.artifact = artifact self.initiators = [initiator_id] self.who = None # we don't know who's going to do this yet - self.running = False - self.failed = False + + self._state = 'queued' + + def describe_state(self): + if self.who is not None: + return self._state + ', given to %s' % self.who + else: + return self._state + + def running(self): + return self._state == 'running' + + def failed(self): + return self._state == 'failed' + + def set_state(self, state): + assert state in ['queued', 'running', 'complete', 'failed'] + logging.debug('Setting job state for job %s with id %s to %s', + self.artifact.basename(), self.id, state) + self._state = state class JobQueue(object): @@ -156,11 +174,8 @@ class JobQueue(object): def __repr__(self): items = [] for job in self._jobs.itervalues(): - if job.who is None: - state = 'queued' - else: - state = 'given to %s' % job.who - items.append('%s (%s)' % (job.artifact.basename(), state)) + items.append( + '%s (%s)' % (job.artifact.basename(), job.describe_state())) return str(items) @@ -244,23 +259,13 @@ class WorkerBuildQueuer(distbuild.StateMachine): self.add_transitions(spec) def _set_job_started(self, event_source, event): - logging.debug('Setting job state for job %s with id %s: ' - 'Job is running', - event.job.artifact.basename(), event.job.id) - - event.job.running = True + event.job.set_state('running') def _set_job_finished(self, event_source, event): - logging.debug('Setting job state for job %s with id %s: ' - 'Job is NOT running', - event.job.artifact.basename(), event.job.id) - - event.job.running = False + event.job.set_state('complete') def _set_job_failed(self, event_source, event): - logging.debug('Job %s with id %s failed', - event.job.artifact.basename(), event.job.id) - event.job.failed = True + event.job.set_state('failed') def _handle_request(self, event_source, event): distbuild.crash_point() @@ -277,7 +282,11 @@ class WorkerBuildQueuer(distbuild.StateMachine): job = self._jobs.get(event.artifact.basename()) job.initiators.append(event.initiator_id) - if job.running: + # Completed jobs are not tracked, so we can't tell here if the + # job was already built. It shouldn't happen, because the + # BuildController has already checked for cached builds when + # annotating the build graph. + if job.running(): logging.debug('Worker build step already started: %s' % event.artifact.basename()) progress = WorkerBuildStepAlreadyStarted(event.initiator_id, @@ -315,7 +324,7 @@ class WorkerBuildQueuer(distbuild.StateMachine): name, job_id) if len(job.initiators) == 1: - if job.running or job.failed: + if job.running() or job.failed(): logging.debug('NOT removing running job %s with job id %s ' '(WorkerConnection will cancel job)', name, job_id) -- cgit v1.2.1