GNOME.org
 

[Notes] [Git][BuildGrid/buildgrid][mablanch/117-job-scheduler-refactoring] 3 commits: job.py: Refactor for stronger encapsulation

Title: GitLab

Martin Blanchard pushed to branch mablanch/117-job-scheduler-refactoring at BuildGrid / buildgrid

Commits:

7 changed files:

Changes:

  • buildgrid/server/bots/instance.py
    ... ... @@ -109,7 +109,8 @@ class BotsInterface:
    109 109 
             if server_state == LeaseState.PENDING:
    110 110
    111 111 
                 if client_state == LeaseState.ACTIVE:
    112 
    -                self._scheduler.update_job_lease_state(client_lease.id, client_lease.state)
    112 
    +                self._scheduler.update_job_lease_state(client_lease.id,
    113 
    +                                                       LeaseState.ACTIVE)
    113 114 
                 elif client_state == LeaseState.COMPLETED:
    114 115 
                     # TODO: Lease was rejected
    115 116 
                     raise NotImplementedError("'Not Accepted' is unsupported")
    ... ... @@ -122,8 +123,10 @@ class BotsInterface:
    122 123 
                     pass
    123 124
    124 125 
                 elif client_state == LeaseState.COMPLETED:
    125 
    -                self._scheduler.update_job_lease_state(client_lease.id, client_lease.state)
    126 
    -                self._scheduler.job_complete(client_lease.id, client_lease.result, client_lease.status)
    126 
    +                self._scheduler.update_job_lease_state(client_lease.id,
    127 
    +                                                       LeaseState.COMPLETED,
    128 
    +                                                       lease_status=client_lease.status,
    129 
    +                                                       lease_result=client_lease.result)
    127 130 
                     return None
    128 131
    129 132 
                 else:

  • buildgrid/server/execution/instance.py
    ... ... @@ -51,9 +51,9 @@ class ExecutionInstance:
    51 51 
             job = Job(action_digest, action.do_not_cache, message_queue)
    52 52 
             self.logger.info("Operation name: [{}]".format(job.name))
    53 53
    54 
    -        self._scheduler.append_job(job, skip_cache_lookup)
    54 
    +        self._scheduler.queue_job(job, skip_cache_lookup)
    55 55
    56 
    -        return job.get_operation()
    56 
    +        return job.operation
    57 57
    58 58 
         def register_message_client(self, name, queue):
    59 59 
             try:

  • buildgrid/server/job.py
    ... ... @@ -11,9 +11,7 @@
    11 11 
     # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    12 12 
     # See the License for the specific language governing permissions and
    13 13 
     # limitations under the License.
    14 
    -#
    15 
    -# Authors:
    16 
    -#        Finn Ball <finn ball codethink co uk>
    14 
    +
    17 15
    18 16 
     import logging
    19 17 
     import uuid
    ... ... @@ -53,18 +51,23 @@ class LeaseState(Enum):
    53 51 
     class Job:
    54 52
    55 53 
         def __init__(self, action_digest, do_not_cache=False, message_queue=None):
    56 
    -        self.lease = None
    57 54 
             self.logger = logging.getLogger(__name__)
    58 
    -        self.n_tries = 0
    59 
    -        self.result = None
    60 
    -        self.result_cached = False
    61 55
    62 
    -        self._action_digest = action_digest
    63 
    -        self._do_not_cache = do_not_cache
    64 
    -        self._execute_stage = OperationStage.UNKNOWN
    65 56 
             self._name = str(uuid.uuid4())
    66 
    -        self._operation = operations_pb2.Operation(name=self._name)
    57 
    +        self._do_not_cache = do_not_cache
    58 
    +        self._operation = operations_pb2.Operation()
    59 
    +        self._lease = None
    60 
    +        self._n_tries = 0
    61 
    +
    62 
    +        self.__execute_response = None
    63 
    +        self.__operation_metadata = remote_execution_pb2.ExecuteOperationMetadata()
    64 
    +
    65 
    +        self.__operation_metadata.action_digest.CopyFrom(action_digest)
    66 
    +        self.__operation_metadata.stage = OperationStage.UNKNOWN.value
    67 
    +
    67 68 
             self._operation_update_queues = []
    69 
    +        self._operation.name = self._name
    70 
    +        self._operation.done = False
    68 71
    69 72 
             if message_queue is not None:
    70 73 
                 self.register_client(message_queue)
    ... ... @@ -73,58 +76,113 @@ class Job:
    73 76 
         def name(self):
    74 77 
             return self._name
    75 78
    79 
    +    @property
    80 
    +    def do_not_cache(self):
    81 
    +        return self._do_not_cache
    82 
    +
    76 83 
         @property
    77 84 
         def action_digest(self):
    78 
    -        return self._action_digest
    85 
    +        return self.__operation_metadata.action_digest
    79 86
    80 87 
         @property
    81 
    -    def do_not_cache(self):
    82 
    -        return self._do_not_cache
    88 
    +    def action_result(self):
    89 
    +        if self.__execute_response is not None:
    90 
    +            return self.__execute_response.result
    91 
    +        else:
    92 
    +            return None
    93 
    +
    94 
    +    @property
    95 
    +    def operation(self):
    96 
    +        return self._operation
    97 
    +
    98 
    +    @property
    99 
    +    def operation_stage(self):
    100 
    +        return OperationStage(self.__operation_metadata.state)
    83 101
    84 
    -    def check_job_finished(self):
    85 
    -        if not self._operation_update_queues:
    86 
    -            return self._operation.done
    87 
    -        return False
    102 
    +    @property
    103 
    +    def lease(self):
    104 
    +        return self._lease
    105 
    +
    106 
    +    @property
    107 
    +    def lease_state(self):
    108 
    +        if self._lease is not None:
    109 
    +            return LeaseState(self._lease.state)
    110 
    +        else:
    111 
    +            return None
    112 
    +
    113 
    +    @property
    114 
    +    def n_tries(self):
    115 
    +        return self._n_tries
    88 116
    89 117 
         def register_client(self, queue):
    90 118 
             self._operation_update_queues.append(queue)
    91 
    -        queue.put(self.get_operation())
    119 
    +        queue.put(self._operation)
    92 120
    93 121 
         def unregister_client(self, queue):
    94 122 
             self._operation_update_queues.remove(queue)
    95 123
    96 
    -    def get_operation(self):
    97 
    -        self._operation.metadata.Pack(self.get_operation_meta())
    98 
    -        if self.result is not None:
    99 
    -            self._operation.done = True
    100 
    -            response = remote_execution_pb2.ExecuteResponse(result=self.result,
    101 
    -                                                            cached_result=self.result_cached)
    124 
    +    def set_cached_result(self, action_result):
    125 
    +        self.__execute_response = remote_execution_pb2.ExecuteResponse()
    126 
    +        self.__execute_response.result.CopyFrom(action_result)
    127 
    +        self.__execute_response.cached_result = True
    102 128
    103 
    -            if not self.result_cached:
    104 
    -                response.status.CopyFrom(self.lease.status)
    129 
    +    def create_lease(self):
    130 
    +        self._lease = bots_pb2.Lease()
    131 
    +        self._lease.id = self._name
    132 
    +        self._lease.payload.Pack(self.__operation_metadata.action_digest)
    133 
    +        self._lease.state = LeaseState.PENDING.value
    105 134
    106 
    -            self._operation.response.Pack(response)
    135 
    +        return self._lease
    107 136
    108 
    -        return self._operation
    137 
    +    def update_lease_state(self, state, status=None, result=None):
    138 
    +        """
    139 
    +        Operates a state transition for the job's current :obj:Lease.
    109 140
    110 
    -    def get_operation_meta(self):
    111 
    -        meta = remote_execution_pb2.ExecuteOperationMetadata()
    112 
    -        meta.stage = self._execute_stage.value
    113 
    -        meta.action_digest.CopyFrom(self._action_digest)
    141 
    +        Args:
    142 
    +            state (LeaseState): the lease state to transition to.
    143 
    +            status (google.rpc.Status): the lease execution status, only
    144 
    +                required if `state` is `COMPLETED`.
    145 
    +            result (google.protobuf.Any): the lease execution result, only
    146 
    +                required if `state` is `COMPLETED`.
    147 
    +        """
    148 
    +        if state.value == self._lease.state:
    149 
    +            return
    114 150
    115 
    -        return meta
    151 
    +        self._lease.state = state.value
    116 152
    117 
    -    def create_lease(self):
    118 
    -        lease = bots_pb2.Lease(id=self.name, state=LeaseState.PENDING.value)
    119 
    -        lease.payload.Pack(self._action_digest)
    153 
    +        if self._lease.state == LeaseState.COMPLETED.value:
    154 
    +            action_result = remote_execution_pb2.ActionResult()
    120 155
    121 
    -        self.lease = lease
    122 
    -        return lease
    156 
    +            if result is not None:
    157 
    +                assert result.Is(action_result.DESCRIPTOR)
    158 
    +                result.Unpack(action_result)
    123 159
    124 
    -    def get_operations(self):
    125 
    -        return operations_pb2.ListOperationsResponse(operations=[self.get_operation()])
    160 
    +            self.__execute_response = remote_execution_pb2.ExecuteResponse()
    161 
    +            self.__execute_response.result.CopyFrom(action_result)
    162 
    +            self.__execute_response.cached_result = False
    163 
    +            self.__execute_response.status.CopyFrom(status)
    126 164
    127 165 
         def update_operation_stage(self, stage):
    128 
    -        self._execute_stage = stage
    166 
    +        """
    167 
    +        Operates a state transition for the job's :obj:Operation.
    168 
    +
    169 
    +        Args:
    170 
    +            stage (OperationStage): the operation stage to transition to.
    171 
    +        """
    172 
    +        if stage.value == self.__operation_metadata.stage:
    173 
    +            return
    174 
    +
    175 
    +        self.__operation_metadata.stage = stage.value
    176 
    +
    177 
    +        if self.__operation_metadata.stage == OperationStage.QUEUED.value:
    178 
    +            self._n_tries += 1
    179 
    +
    180 
    +        elif self.__operation_metadata.stage == OperationStage.COMPLETED.value:
    181 
    +            if self.__execute_response is not None:
    182 
    +                self._operation.response.Pack(self.__execute_response)
    183 
    +            self._operation.done = True
    184 
    +
    185 
    +        self._operation.metadata.Pack(self.__operation_metadata)
    186 
    +
    129 187 
             for queue in self._operation_update_queues:
    130 
    -            queue.put(self.get_operation())
    188 
    +            queue.put(self._operation)

  • buildgrid/server/operations/instance.py
    ... ... @@ -22,6 +22,7 @@ An instance of the LongRunningOperations Service.
    22 22 
     import logging
    23 23
    24 24 
     from buildgrid._exceptions import InvalidArgumentError
    25 
    +from buildgrid._protos.google.longrunning import operations_pb2
    25 26
    26 27
    27 28 
     class OperationsInstance:
    ... ... @@ -34,18 +35,21 @@ class OperationsInstance:
    34 35 
             server.add_operations_instance(self, instance_name)
    35 36
    36 37 
         def get_operation(self, name):
    37 
    -        operation = self._scheduler.jobs.get(name)
    38 
    +        job = self._scheduler.jobs.get(name)
    38 39
    39 
    -        if operation is None:
    40 
    +        if job is None:
    40 41 
                 raise InvalidArgumentError("Operation name does not exist: [{}]".format(name))
    41 42
    42 43 
             else:
    43 
    -            return operation.get_operation()
    44 
    +            return job.operation
    44 45
    45 46 
         def list_operations(self, list_filter, page_size, page_token):
    46 47 
             # TODO: Pages
    47 48 
             # Spec says number of pages and length of a page are optional
    48 
    -        return self._scheduler.get_operations()
    49 
    +        response = operations_pb2.ListOperationsResponse()
    50 
    +        response.operations.extend(self._scheduler.list_operations())
    51 
    +
    52 
    +        return response
    49 53
    50 54 
         def delete_operation(self, name):
    51 55 
             try:

  • buildgrid/server/scheduler.py
    ... ... @@ -11,9 +11,7 @@
    11 11 
     # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    12 12 
     # See the License for the specific language governing permissions and
    13 13 
     # limitations under the License.
    14 
    -#
    15 
    -# Authors:
    16 
    -#        Finn Ball <finn ball codethink co uk>
    14 
    +
    17 15
    18 16 
     """
    19 17 
     Scheduler
    ... ... @@ -24,8 +22,6 @@ Schedules jobs.
    24 22 
     from collections import deque
    25 23
    26 24 
     from buildgrid._exceptions import NotFoundError
    27 
    -from buildgrid._protos.build.bazel.remote.execution.v2 import remote_execution_pb2
    28 
    -from buildgrid._protos.google.longrunning import operations_pb2
    29 25
    30 26 
     from .job import OperationStage, LeaseState
    31 27
    ... ... @@ -39,80 +35,73 @@ class Scheduler:
    39 35 
             self.jobs = {}
    40 36 
             self.queue = deque()
    41 37
    42 
    -    def register_client(self, name, queue):
    43 
    -        self.jobs[name].register_client(queue)
    38 
    +    def register_client(self, job_name, queue):
    39 
    +        self.jobs[job_name].register_client(queue)
    44 40
    45 
    -    def unregister_client(self, name, queue):
    46 
    -        job = self.jobs[name]
    47 
    -        job.unregister_client(queue)
    48 
    -        if job.check_job_finished():
    49 
    -            del self.jobs[name]
    41 
    +    def unregister_client(self, job_name, queue):
    42 
    +        self.jobs[job_name].unregister_client(queue)
    50 43
    51 
    -    def append_job(self, job, skip_cache_lookup=False):
    44 
    +        if self.jobs[job_name].operation.done:
    45 
    +            del self.jobs[job_name]
    46 
    +
    47 
    +    def queue_job(self, job, skip_cache_lookup=False):
    52 48 
             self.jobs[job.name] = job
    49 
    +
    50 
    +        operation_stage = None
    53 51 
             if self._action_cache is not None and not skip_cache_lookup:
    54 52 
                 try:
    55 53 
                     cached_result = self._action_cache.get_action_result(job.action_digest)
    56 54 
                 except NotFoundError:
    55 
    +                operation_stage = OperationStage.QUEUED
    57 56 
                     self.queue.append(job)
    58 
    -                job.update_operation_stage(OperationStage.QUEUED)
    59 57
    60 58 
                 else:
    61 
    -                job.result = cached_result
    62 
    -                job.result_cached = True
    63 
    -                job.update_operation_stage(OperationStage.COMPLETED)
    59 
    +                job.set_cached_result(cached_result)
    60 
    +                operation_stage = OperationStage.COMPLETED
    64 61
    65 62 
             else:
    63 
    +            operation_stage = OperationStage.QUEUED
    66 64 
                 self.queue.append(job)
    67 
    -            job.update_operation_stage(OperationStage.QUEUED)
    68 65
    69 
    -    def retry_job(self, name):
    70 
    -        if name in self.jobs:
    71 
    -            job = self.jobs[name]
    66 
    +        job.update_operation_stage(operation_stage)
    67 
    +
    68 
    +    def retry_job(self, job_name):
    69 
    +        if job_name in self.jobs:
    70 
    +            job = self.jobs[job_name]
    72 71 
                 if job.n_tries >= self.MAX_N_TRIES:
    73 72 
                     # TODO: Decide what to do with these jobs
    74 73 
                     job.update_operation_stage(OperationStage.COMPLETED)
    75 74 
                     # TODO: Mark these jobs as done
    76 75 
                 else:
    77 76 
                     job.update_operation_stage(OperationStage.QUEUED)
    78 
    -                job.n_tries += 1
    79 77 
                     self.queue.appendleft(job)
    80 78
    81 
    -    def job_complete(self, name, result, status):
    82 
    -        job = self.jobs[name]
    83 
    -        job.lease.status.CopyFrom(status)
    84 
    -        action_result = remote_execution_pb2.ActionResult()
    85 
    -        result.Unpack(action_result)
    86 
    -        job.result = action_result
    87 
    -        if not job.do_not_cache and self._action_cache is not None:
    88 
    -            if not job.lease.status.code:
    89 
    -                self._action_cache.update_action_result(job.action_digest, action_result)
    90 
    -        job.update_operation_stage(OperationStage.COMPLETED)
    91 
    -
    92 
    -    def get_operations(self):
    93 
    -        response = operations_pb2.ListOperationsResponse()
    94 
    -        for v in self.jobs.values():
    95 
    -            response.operations.extend([v.get_operation()])
    96 
    -        return response
    97 
    -
    98 
    -    def update_job_lease_state(self, name, state):
    99 
    -        job = self.jobs[name]
    100 
    -        job.lease.state = state
    101 
    -
    102 
    -    def get_job_lease(self, name):
    103 
    -        return self.jobs[name].lease
    104 
    -
    105 
    -    def cancel_session(self, name):
    106 
    -        job = self.jobs[name]
    107 
    -        state = job.lease.state
    108 
    -        if state in (LeaseState.PENDING.value, LeaseState.ACTIVE.value):
    109 
    -            self.retry_job(name)
    79 
    +    def list_operations(self):
    80 
    +        return [job.operation for job in self.jobs.values()]
    81 
    +
    82 
    +    def update_job_lease_state(self, job_name, lease_state, lease_status=None, lease_result=None):
    83 
    +        job = self.jobs[job_name]
    84 
    +        if lease_state != LeaseState.COMPLETED:
    85 
    +            job.update_lease_state(lease_state)
    86 
    +        else:
    87 
    +            job.update_lease_state(lease_state, status=lease_status, result=lease_result)
    88 
    +
    89 
    +            if not job.do_not_cache and self._action_cache is not None:
    90 
    +                if not job.lease.status.code:
    91 
    +                    self._action_cache.update_action_result(job.action_digest, job.action_result)
    92 
    +
    93 
    +            job.update_operation_stage(OperationStage.COMPLETED)
    94 
    +
    95 
    +    def get_job_lease(self, job_name):
    96 
    +        return self.jobs[job_name].lease
    110 97
    111 98 
         def create_lease(self):
    112 99 
             if self.queue:
    113 100 
                 job = self.queue.popleft()
    114 101 
                 job.update_operation_stage(OperationStage.EXECUTING)
    115 102 
                 job.create_lease()
    116 
    -            job.lease.state = LeaseState.PENDING.value
    117 103 
                 return job.lease
    118 104 
             return None
    105 
    +
    106 
    +    def get_job_operation(self, job_name):
    107 
    +        return self.jobs[job_name].operation

  • tests/integration/bots_service.py
    ... ... @@ -174,6 +174,7 @@ def test_update_leases_work_complete(bot_session, context, instance):
    174 174 
         response = copy.deepcopy(instance.UpdateBotSession(request, context))
    175 175
    176 176 
         response.leases[0].state = LeaseState.COMPLETED.value
    177 
    +    response.leases[0].result.Pack(remote_execution_pb2.ActionResult())
    177 178
    178 179 
         request = bots_pb2.UpdateBotSessionRequest(name=response.name,
    179 180 
                                                    bot_session=response)
    ... ... @@ -283,4 +284,4 @@ def _inject_work(scheduler, action_digest=None):
    283 284 
         if not action_digest:
    284 285 
             action_digest = remote_execution_pb2.Digest()
    285 286 
         j = job.Job(action_digest, False)
    286 
    -    scheduler.append_job(j, True)
    287 
    +    scheduler.queue_job(j, True)

  • tests/integration/operations_service.py
    ... ... @@ -30,6 +30,7 @@ from buildgrid._protos.google.longrunning import operations_pb2
    30 30 
     from buildgrid._protos.google.rpc import status_pb2
    31 31 
     from buildgrid.server.cas.storage import lru_memory_cache
    32 32 
     from buildgrid.server.controller import ExecutionController
    33 
    +from buildgrid.server.job import LeaseState
    33 34 
     from buildgrid.server.operations import service
    34 35 
     from buildgrid.server.operations.service import OperationsService
    35 36 
     from buildgrid.utils import create_digest
    ... ... @@ -131,9 +132,10 @@ def test_list_operations_with_result(instance, controller, execute_request, cont
    131 132 
         action_result.output_files.extend([output_file])
    132 133
    133 134 
         controller.operations_instance._scheduler.jobs[response_execute.name].create_lease()
    134 
    -    controller.operations_instance._scheduler.job_complete(response_execute.name,
    135 
    -                                                           _pack_any(action_result),
    136 
    -                                                           status_pb2.Status())
    135 
    +    controller.operations_instance._scheduler.update_job_lease_state(response_execute.name,
    136 
    +                                                                     LeaseState.COMPLETED,
    137 
    +                                                                     lease_status=status_pb2.Status(),
    138 
    +                                                                     lease_result=_pack_any(action_result))
    137 139
    138 140 
         request = operations_pb2.ListOperationsRequest(name=instance_name)
    139 141 
         response = instance.ListOperations(request, context)

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.

    [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]