diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -69,40 +69,25 @@ /** - * @task lease - */ - final public function filterResource( - DrydockResource $resource, - DrydockLease $lease) { - - $scope = $this->pushActiveScope($resource, $lease); - - return $this->canAllocateLease($resource, $lease); - } - - - /** * Enforce basic checks on lease/resource compatibility. Allows resources to * reject leases if they are incompatible, even if the resource types match. * * For example, if a resource represents a 32-bit host, this method might - * reject leases that need a 64-bit host. If a resource represents a working - * copy of repository "X", this method might reject leases which need a - * working copy of repository "Y". Generally, although the main types of - * a lease and resource may match (e.g., both "host"), it may not actually be - * possible to satisfy the lease with a specific resource. + * reject leases that need a 64-bit host. The blueprint might also reject + * a resource if the lease needs 8GB of RAM and the resource only has 6GB + * free. * - * This method generally should not enforce limits or perform capacity - * checks. Perform those in @{method:shouldAllocateLease} instead. It also - * should not perform actual acquisition of the lease; perform that in - * @{method:executeAcquireLease} instead. + * This method should not acquire locks or expect anything to be locked. This + * is a coarse compatibility check between a lease and a resource. * - * @param DrydockResource Candidiate resource to allocate the lease on. - * @param DrydockLease Pending lease that wants to allocate here. - * @return bool True if the resource and lease are compatible. + * @param DrydockBlueprint Concrete blueprint to allocate for. + * @param DrydockResource Candidiate resource to allocate the lease on. + * @param DrydockLease Pending lease that wants to allocate here. + * @return bool True if the resource and lease are compatible. * @task lease */ - abstract protected function canAllocateLease( + abstract public function canAllocateLeaseOnResource( + DrydockBlueprint $blueprint, DrydockResource $resource, DrydockLease $lease); @@ -297,14 +282,102 @@ /* -( Resource Allocation )------------------------------------------------ */ - public function canAllocateMoreResources(array $pool) { - return true; - } + /** + * Enforce fundamental implementation/lease checks. Allows implementations to + * reject a lease which no concrete blueprint can ever satisfy. + * + * For example, if a lease only builds ARM hosts and the lease needs a + * PowerPC host, it may be rejected here. + * + * This is the earliest rejection phase, and followed by + * @{method:canEverAllocateResourceForLease}. + * + * This method should not actually check if a resource can be allocated + * right now, or even if a blueprint which can allocate a suitable resource + * really exists, only if some blueprint may conceivably exist which could + * plausibly be able to build a suitable resource. + * + * @param DrydockLease Requested lease. + * @return bool True if some concrete blueprint of this implementation's + * type might ever be able to build a resource for the lease. + * @task resource + */ + abstract public function canAnyBlueprintEverAllocateResourceForLease( + DrydockLease $lease); + + + /** + * Enforce basic blueprint/lease checks. Allows blueprints to reject a lease + * which they can not build a resource for. + * + * This is the second rejection phase. It follows + * @{method:canAnyBlueprintEverAllocateResourceForLease} and is followed by + * @{method:canAllocateResourceForLease}. + * + * This method should not check if a resource can be built right now, only + * if the blueprint as configured may, at some time, be able to build a + * suitable resource. + * + * @param DrydockBlueprint Blueprint which may be asked to allocate a + * resource. + * @param DrydockLease Requested lease. + * @return bool True if this blueprint can eventually build a suitable + * resource for the lease, as currently configured. + * @task resource + */ + abstract public function canEverAllocateResourceForLease( + DrydockBlueprint $blueprint, + DrydockLease $lease); + - abstract protected function executeAllocateResource(DrydockLease $lease); + /** + * Enforce basic availability limits. Allows blueprints to reject resource + * allocation if they are currently overallocated. + * + * This method should perform basic capacity/limit checks. For example, if + * it has a limit of 6 resources and currently has 6 resources allocated, + * it might reject new leases. + * + * This method should not acquire locks or expect locks to be acquired. This + * is a coarse check to determine if the operation is likely to succeed + * right now without needing to acquire locks. + * + * It is expected that this method will sometimes return `true` (indicating + * that a resource can be allocated) but find that another allocator has + * eaten up free capacity by the time it actually tries to build a resource. + * This is normal and the allocator will recover from it. + * + * @param DrydockBlueprint The blueprint which may be asked to allocate a + * resource. + * @param DrydockLease Requested lease. + * @return bool True if this blueprint appears likely to be able to allocate + * a suitable resource. + */ + abstract public function canAllocateResourceForLease( + DrydockBlueprint $blueprint, + DrydockLease $lease); - final public function allocateResource(DrydockLease $lease) { + /** + * Allocate a suitable resource for a lease. + * + * This method MUST acquire, hold, and manage locks to prevent multiple + * allocations from racing. World state is not locked before this method is + * called. Blueprints are entirely responsible for any lock handling they + * need to perform. + * + * @param DrydockBlueprint The blueprint which should allocate a resource. + * @param DrydockLease Requested lease. + * @return DrydockResource Allocated resource. + */ + abstract protected function executeAllocateResource( + DrydockBlueprint $blueprint, + DrydockLease $lease); + + final public function allocateResource( + DrydockBlueprint $blueprint, + DrydockLease $lease) { + $scope = $this->pushActiveScope(null, $lease); $this->log( @@ -314,7 +387,7 @@ $lease->getLeaseName())); try { - $resource = $this->executeAllocateResource($lease); + $resource = $this->executeAllocateResource($blueprint, $lease); $this->validateAllocatedResource($resource); } catch (Exception $ex) { $this->logException($ex); @@ -377,14 +450,6 @@ ->execute(); } - public static function getAllBlueprintImplementationsForResource($type) { - static $groups = null; - if ($groups === null) { - $groups = mgroup(self::getAllBlueprintImplementations(), 'getType'); - } - return idx($groups, $type, array()); - } - public static function getNamedImplementation($class) { return idx(self::getAllBlueprintImplementations(), $class); } diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -15,9 +15,31 @@ return pht('Allows Drydock to check out working copies of repositories.'); } - protected function canAllocateLease( + public function canAnyBlueprintEverAllocateResourceForLease( + DrydockLease $lease) { + // TODO: These checks are out of date. + return true; + } + + public function canEverAllocateResourceForLease( + DrydockBlueprint $blueprint, + DrydockLease $lease) { + // TODO: These checks are out of date. + return true; + } + + public function canAllocateResourceForLease( + DrydockBlueprint $blueprint, + DrydockLease $lease) { + // TODO: These checks are out of date. + return true; + } + + public function canAllocateLeaseOnResource( + DrydockBlueprint $blueprint, DrydockResource $resource, DrydockLease $lease) { + // TODO: These checks are out of date. $resource_repo = $resource->getAttribute('repositoryID'); $lease_repo = $lease->getAttribute('repositoryID'); @@ -29,11 +51,14 @@ DrydockResource $resource, DrydockLease $lease, array $other_leases) { - + // TODO: These checks are out of date. return !$other_leases; } - protected function executeAllocateResource(DrydockLease $lease) { + protected function executeAllocateResource( + DrydockBlueprint $blueprint, + DrydockLease $lease) { + $repository_id = $lease->getAttribute('repositoryID'); if (!$repository_id) { throw new Exception( diff --git a/src/applications/drydock/storage/DrydockBlueprint.php b/src/applications/drydock/storage/DrydockBlueprint.php --- a/src/applications/drydock/storage/DrydockBlueprint.php +++ b/src/applications/drydock/storage/DrydockBlueprint.php @@ -51,16 +51,7 @@ } public function getImplementation() { - $class = $this->className; - $implementations = - DrydockBlueprintImplementation::getAllBlueprintImplementations(); - if (!isset($implementations[$class])) { - throw new Exception( - pht( - "Invalid class name for blueprint (got '%s')", - $class)); - } - return id(new $class())->attachInstance($this); + return $this->assertAttached($this->implementation); } public function attachImplementation(DrydockBlueprintImplementation $impl) { @@ -77,6 +68,26 @@ return $this; } + public function canEverAllocateResourceForLease(DrydockLease $lease) { + return $this->getImplementation()->canEverAllocateResourceForLease( + $this, + $lease); + } + + public function canAllocateResourceForLease(DrydockLease $lease) { + return $this->getImplementation()->canAllocateResourceForLease( + $this, + $lease); + } + + public function canAllocateLeaseOnResource( + DrydockResource $resource, + DrydockLease $lease) { + return $this->getImplementation()->canAllocateLeaseOnResource( + $this, + $resource, + $lease); + } /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/drydock/worker/DrydockAllocatorWorker.php b/src/applications/drydock/worker/DrydockAllocatorWorker.php --- a/src/applications/drydock/worker/DrydockAllocatorWorker.php +++ b/src/applications/drydock/worker/DrydockAllocatorWorker.php @@ -2,186 +2,310 @@ final class DrydockAllocatorWorker extends PhabricatorWorker { - private $lease; - - public function getRequiredLeaseTime() { - return 3600 * 24; - } - - public function getMaximumRetryCount() { - // TODO: Allow Drydock allocations to retry. For now, every failure is - // permanent and most of them are because I am bad at programming, so fail - // fast rather than ending up in limbo. - return 0; + private function getViewer() { + return PhabricatorUser::getOmnipotentUser(); } private function loadLease() { - if (empty($this->lease)) { - $lease = id(new DrydockLeaseQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withIDs(array($this->getTaskData())) - ->executeOne(); - if (!$lease) { - throw new PhabricatorWorkerPermanentFailureException( - pht('No such lease %d!', $this->getTaskData())); - } - $this->lease = $lease; + $viewer = $this->getViewer(); + + // TODO: Make the task data a dictionary like every other worker, and + // probably make this a PHID. + $lease_id = $this->getTaskData(); + + $lease = id(new DrydockLeaseQuery()) + ->setViewer($viewer) + ->withIDs(array($lease_id)) + ->executeOne(); + if (!$lease) { + throw new PhabricatorWorkerPermanentFailureException( + pht('No such lease "%s"!', $lease_id)); } - return $this->lease; - } - private function logToDrydock($message) { - DrydockBlueprintImplementation::writeLog( - null, - $this->loadLease(), - $message); + return $lease; } protected function doWork() { $lease = $this->loadLease(); - $this->logToDrydock(pht('Allocating Lease')); - - try { - $this->allocateLease($lease); - } catch (Exception $ex) { - - // TODO: We should really do this when archiving the task, if we've - // suffered a permanent failure. But we don't have hooks for that yet - // and always fail after the first retry right now, so this is - // functionally equivalent. - $lease->reload(); - if ($lease->getStatus() == DrydockLeaseStatus::STATUS_PENDING) { - $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); - $lease->save(); + $this->allocateLease($lease); + } + + private function allocateLease(DrydockLease $lease) { + $blueprints = $this->loadBlueprintsForAllocatingLease($lease); + + // If we get nothing back, that means no blueprint is defined which can + // ever build the requested resource. This is a permanent failure, since + // we don't expect to succeed no matter how many times we try. + if (!$blueprints) { + $lease + ->setStatus(DrydockLeaseStatus::STATUS_BROKEN) + ->save(); + throw new PhabricatorWorkerPermanentFailureException( + pht( + 'No active Drydock blueprint exists which can ever allocate a '. + 'resource for lease "%s".', + $lease->getPHID())); + } + + // First, try to find a suitable open resource which we can acquire a new + // lease on. + $resources = $this->loadResourcesForAllocatingLease($blueprints, $lease); + + // If no resources exist yet, see if we can build one. + if (!$resources) { + $usable_blueprints = $this->removeOverallocatedBlueprints( + $blueprints, + $lease); + + // If we get nothing back here, some blueprint claims it can eventually + // satisfy the lease, just not right now. This is a temporary failure, + // and we expect allocation to succeed eventually. + if (!$blueprints) { + // TODO: More formal temporary failure here. We should retry this + // "soon" but not "immediately". + throw new Exception( + pht('No blueprints have space to allocate a resource right now.')); + } + + $usable_blueprints = $this->rankBlueprints($blueprints, $lease); + + $exceptions = array(); + foreach ($usable_blueprints as $blueprint) { + try { + $resources[] = $blueprint->allocateResource($lease); + // Bail after allocating one resource, we don't need any more than + // this. + break; + } catch (Exception $ex) { + $exceptions[] = $ex; + } } - throw $ex; + if (!$resources) { + // TODO: We should distinguish between temporary and permament failures + // here. If any blueprint failed temporarily, retry "soon". If none + // of these failures were temporary, maybe this should be a permanent + // failure? + throw new PhutilAggregateException( + pht( + 'All blueprints failed to allocate a suitable new resource when '. + 'trying to allocate lease "%s".', + $lease->getPHID()), + $exceptions); + } + + // NOTE: We have not acquired the lease yet, so it is possible that the + // resource we just built will be snatched up by some other lease before + // we can. This is not problematic: we'll retry a little later and should + // suceed eventually. + } + + $resources = $this->rankResources($resources, $lease); + + $exceptions = array(); + $allocated = false; + foreach ($resources as $resource) { + try { + $blueprint->allocateLease($resource, $lease); + $allocated = true; + break; + } catch (Exception $ex) { + $exceptions[] = $ex; + } + } + + if (!$allocated) { + // TODO: We should distinguish between temporary and permanent failures + // here. If any failures were temporary (specifically, failed to acquire + // locks) + + throw new PhutilAggregateException( + pht( + 'Unable to acquire lease "%s" on any resouce.', + $lease->getPHID()), + $exceptions); } } - private function loadAllBlueprints() { - $viewer = PhabricatorUser::getOmnipotentUser(); - $instances = id(new DrydockBlueprintQuery()) + + /** + * Load a list of all resources which a given lease can possibly be + * allocated against. + * + * @param list Blueprints which may produce suitable + * resources. + * @param DrydockLease Requested lease. + * @return list Resources which may be able to allocate + * the lease. + */ + private function loadResourcesForAllocatingLease( + array $blueprints, + DrydockLease $lease) { + assert_instances_of($blueprints, 'DrydockBlueprint'); + $viewer = $this->getViewer(); + + $resources = id(new DrydockResourceQuery()) ->setViewer($viewer) + ->withBlueprintPHIDs(mpull($blueprints, 'getPHID')) + ->withTypes(array($lease->getResourceType())) + ->withStatuses( + array( + DrydockResourceStatus::STATUS_PENDING, + DrydockResourceStatus::STATUS_OPEN, + )) ->execute(); - $blueprints = array(); - foreach ($instances as $instance) { - $blueprints[$instance->getPHID()] = $instance; + + $keep = array(); + foreach ($resources as $key => $resource) { + if (!$resource->canAllocateLease($lease)) { + continue; + } + + $keep[$key] = $resource; } - return $blueprints; + + return $keep; } - private function allocateLease(DrydockLease $lease) { - $type = $lease->getResourceType(); - $blueprints = $this->loadAllBlueprints(); + /** + * Rank blueprints by suitability for building a new resource for a + * particular lease. + * + * @param list List of blueprints. + * @param DrydockLease Requested lease. + * @return list Ranked list of blueprints. + */ + private function rankBlueprints(array $blueprints, DrydockLease $lease) { + assert_instances_of($blueprints, 'DrydockBlueprint'); - // TODO: Policy stuff. - $pool = id(new DrydockResource())->loadAllWhere( - 'type = %s AND status = %s', - $lease->getResourceType(), - DrydockResourceStatus::STATUS_OPEN); + // TODO: Implement improvements to this ranking algorithm if they become + // available. + shuffle($blueprints); - $this->logToDrydock( - pht('Found %d Open Resource(s)', count($pool))); + return $blueprints; + } - $candidates = array(); - foreach ($pool as $key => $candidate) { - if (!isset($blueprints[$candidate->getBlueprintPHID()])) { - unset($pool[$key]); - continue; - } - $blueprint = $blueprints[$candidate->getBlueprintPHID()]; - $implementation = $blueprint->getImplementation(); + /** + * Rank resources by suitability for allocating a particular lease. + * + * @param list List of resources. + * @param DrydockLease Requested lease. + * @return list Ranked list of resources. + */ + private function rankResources(array $resources, DrydockLease $lease) { + assert_instances_of($resources, 'DrydockResource'); - if ($implementation->filterResource($candidate, $lease)) { - $candidates[] = $candidate; - } + // TODO: Implement improvements to this ranking algorithm if they become + // available. + shuffle($resources); + + return $resources; + } + + + /** + * Get all the concrete @{class:DrydockBlueprint}s which can possibly + * build a resource to satisfy a lease. + * + * @param DrydockLease Requested lease. + * @return list List of qualifying blueprints. + */ + private function loadBlueprintsForAllocatingLease( + DrydockLease $lease) { + $viewer = $this->getViewer(); + + $impls = $this->loadBlueprintImplementationsForAllocatingLease($lease); + if (!$impls) { + return array(); } - $this->logToDrydock(pht('%d Open Resource(s) Remain', count($candidates))); + // TODO: When blueprints can be disabled, this query should ignore disabled + // blueprints. - $resource = null; - if ($candidates) { - shuffle($candidates); - foreach ($candidates as $candidate_resource) { - $blueprint = $blueprints[$candidate_resource->getBlueprintPHID()] - ->getImplementation(); - if ($blueprint->allocateLease($candidate_resource, $lease)) { - $resource = $candidate_resource; - break; - } + $blueprints = id(new DrydockBlueprintQuery()) + ->setViewer($viewer) + ->withBlueprintClasses(array_keys($impls)) + ->execute(); + + $keep = array(); + foreach ($blueprints as $key => $blueprint) { + if (!$blueprint->canEverAllocateResourceForLease($lease)) { + continue; } + + $keep[$key] = $blueprint; } - if (!$resource) { - $blueprints = DrydockBlueprintImplementation - ::getAllBlueprintImplementationsForResource($type); + return $keep; + } - $this->logToDrydock( - pht('Found %d Blueprints', count($blueprints))); - foreach ($blueprints as $key => $candidate_blueprint) { - if (!$candidate_blueprint->isEnabled()) { - unset($blueprints[$key]); - continue; - } + /** + * Get all the @{class:DrydockBlueprintImplementation}s which can possibly + * build a resource to satisfy a lease. + * + * This method returns blueprints which might, at some time, be able to + * build a resource which can satisfy the lease. They may not be able to + * build that resource right now. + * + * @param DrydockLease Requested lease. + * @return list List of qualifying blueprint + * implementations. + */ + private function loadBlueprintImplementationsForAllocatingLease( + DrydockLease $lease) { + + $impls = DrydockBlueprintImplementation::getAllBlueprintImplementations(); + + $keep = array(); + foreach ($impls as $key => $impl) { + // Don't use disabled blueprint types. + if (!$impl->isEnabled()) { + continue; } - $this->logToDrydock( - pht('%d Blueprints Enabled', count($blueprints))); + // Don't use blueprint types which can't allocate the correct kind of + // resource. + if ($impl->getType() != $lease->getResourceType()) { + continue; + } - foreach ($blueprints as $key => $candidate_blueprint) { - if (!$candidate_blueprint->canAllocateMoreResources($pool)) { - unset($blueprints[$key]); - continue; - } + if (!$impl->canAnyBlueprintEverAllocateResourceForLease($lease)) { + continue; } - $this->logToDrydock( - pht('%d Blueprints Can Allocate', count($blueprints))); + $keep[$key] = $impl; + } - if (!$blueprints) { - $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); - $lease->save(); + return $keep; + } - $this->logToDrydock( - pht( - "There are no resources of type '%s' available, and no ". - "blueprints which can allocate new ones.", - $type)); - return; + /** + * Remove blueprints which are too heavily allocated to build a resource for + * a lease from a list of blueprints. + * + * @param list List of blueprints. + * @param list List with fully allocated blueprints + * removed. + */ + private function removeOverallocatedBlueprints( + array $blueprints, + DrydockLease $lease) { + assert_instances_of($blueprints, 'DrydockBlueprint'); + + $keep = array(); + foreach ($blueprints as $key => $blueprint) { + if (!$blueprint->canAllocateResourceForLease($lease)) { + continue; } - // TODO: Rank intelligently. - shuffle($blueprints); - - $blueprint = head($blueprints); - $resource = $blueprint->allocateResource($lease); - - if (!$blueprint->allocateLease($resource, $lease)) { - // TODO: This "should" happen only if we lost a race with another lease, - // which happened to acquire this resource immediately after we - // allocated it. In this case, the right behavior is to retry - // immediately. However, other things like a blueprint allocating a - // resource it can't actually allocate the lease on might be happening - // too, in which case we'd just allocate infinite resources. Probably - // what we should do is test for an active or allocated lease and retry - // if we find one (although it might have already been released by now) - // and fail really hard ("your configuration is a huge broken mess") - // otherwise. But just throw for now since this stuff is all edge-casey. - // Alternatively we could bring resources up in a "BESPOKE" status - // and then switch them to "OPEN" only after the allocating lease gets - // its grubby mitts on the resource. This might make more sense but - // is a bit messy. - throw new Exception(pht('Lost an allocation race?')); - } + $keep[$key] = $blueprint; } - $blueprint = $resource->getBlueprint(); - $blueprint->acquireLease($resource, $lease); + return $keep; } }