diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -791,6 +791,7 @@ 'DoorkeeperSchemaSpec' => 'applications/doorkeeper/storage/DoorkeeperSchemaSpec.php', 'DoorkeeperTagView' => 'applications/doorkeeper/view/DoorkeeperTagView.php', 'DoorkeeperTagsController' => 'applications/doorkeeper/controller/DoorkeeperTagsController.php', + 'DrydockAllocationContext' => 'applications/drydock/util/DrydockAllocationContext.php', 'DrydockAllocatorWorker' => 'applications/drydock/worker/DrydockAllocatorWorker.php', 'DrydockApacheWebrootInterface' => 'applications/drydock/interface/webroot/DrydockApacheWebrootInterface.php', 'DrydockBlueprint' => 'applications/drydock/storage/DrydockBlueprint.php', @@ -842,6 +843,8 @@ 'DrydockManagementLeaseWorkflow' => 'applications/drydock/management/DrydockManagementLeaseWorkflow.php', 'DrydockManagementReleaseWorkflow' => 'applications/drydock/management/DrydockManagementReleaseWorkflow.php', 'DrydockManagementWorkflow' => 'applications/drydock/management/DrydockManagementWorkflow.php', + 'DrydockMinMaxBlueprintImplementation' => 'applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php', + 'DrydockMinMaxExpiryBlueprintImplementation' => 'applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php', 'DrydockPreallocatedHostBlueprintImplementation' => 'applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php', 'DrydockQuery' => 'applications/drydock/query/DrydockQuery.php', 'DrydockResource' => 'applications/drydock/storage/DrydockResource.php', @@ -4464,6 +4467,7 @@ 'DoorkeeperSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'DoorkeeperTagView' => 'AphrontView', 'DoorkeeperTagsController' => 'PhabricatorController', + 'DrydockAllocationContext' => 'Phobject', 'DrydockAllocatorWorker' => 'PhabricatorWorker', 'DrydockApacheWebrootInterface' => 'DrydockWebrootInterface', 'DrydockBlueprint' => array( @@ -4529,6 +4533,8 @@ 'DrydockManagementLeaseWorkflow' => 'DrydockManagementWorkflow', 'DrydockManagementReleaseWorkflow' => 'DrydockManagementWorkflow', 'DrydockManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'DrydockMinMaxBlueprintImplementation' => 'DrydockBlueprintImplementation', + 'DrydockMinMaxExpiryBlueprintImplementation' => 'DrydockMinMaxBlueprintImplementation', 'DrydockPreallocatedHostBlueprintImplementation' => 'DrydockBlueprintImplementation', 'DrydockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DrydockResource' => array( 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 @@ -19,6 +19,10 @@ abstract public function isEnabled(); + public function isTest() { + return false; + } + abstract public function getBlueprintName(); abstract public function getDescription(); @@ -42,7 +46,7 @@ return $lease; } - protected function getInstance() { + public function getInstance() { if (!$this->instance) { throw new Exception( pht('Attach the blueprint instance to the implementation.')); @@ -127,6 +131,11 @@ $allocated = false; $allocation_exception = null; + $context = new DrydockAllocationContext( + $this->getInstance(), + $resource, + $lease); + $resource->openTransaction(); $resource->beginReadLocking(); $resource->reload(); @@ -142,9 +151,9 @@ try { $allocated = $this->shouldAllocateLease( + $context, $resource, - $ephemeral_lease, - $other_leases); + $ephemeral_lease); } catch (Exception $ex) { $allocation_exception = $ex; } @@ -197,19 +206,15 @@ * better implemented in @{method:canAllocateLease}, which serves as a * cheap filter before lock acquisition. * + * @param DrydockAllocationContext Relevant contextual information. * @param DrydockResource Candidate resource to allocate the lease on. * @param DrydockLease Pending lease that wants to allocate here. - * @param list Other allocated and acquired leases on the - * resource. The implementation can inspect them - * to verify it can safely add the new lease. - * @return bool True to allocate the lease on the resource; - * false to reject it. * @task lease */ abstract protected function shouldAllocateLease( + DrydockAllocationContext $context, DrydockResource $resource, - DrydockLease $lease, - array $other_leases); + DrydockLease $lease); /** @@ -265,10 +270,25 @@ DrydockLease $lease); + /** + * Release an allocated lease, performing any desired cleanup. + * + * After this method executes, the lease status is moved to `RELEASED`. + * + * If release fails, throw an exception. + * + * @param DrydockResource Resource to release the lease from. + * @param DrydockLease Lease to release. + * @return void + */ + abstract protected function executeReleaseLease( + DrydockResource $resource, + DrydockLease $lease); final public function releaseLease( DrydockResource $resource, - DrydockLease $lease) { + DrydockLease $lease, + $caused_by_closing_resource = false) { $scope = $this->pushActiveScope(null, $lease); $released = false; @@ -286,10 +306,36 @@ $lease->endReadLocking(); $lease->saveTransaction(); + // Execute clean up outside of the lock and don't perform clean up if the + // resource is closing anyway, because in that scenario, the closing + // resource will clean up all the leases anyway (e.g. an EC2 host being + // terminated that contains leases on it's instance storage). + if ($released && !$caused_by_closing_resource) { + $this->executeReleaseLease($resource, $lease); + } + if (!$released) { throw new Exception(pht('Unable to release lease: lease not active!')); } + if (!$caused_by_closing_resource) { + // Check to see if the resource has no more leases, and if so, ask the + // blueprint as to whether this resource should be closed. + $context = new DrydockAllocationContext( + $this->getInstance(), + $resource, + $lease); + + if ($context->getCurrentResourceLeaseCount() === 0) { + if ($this->shouldCloseUnleasedResource($context, $resource)) { + self::writeLog( + $resource, + null, + pht('Closing resource because it has no more active leases')); + $this->closeResource($resource); + } + } + } } @@ -301,10 +347,106 @@ return true; } - abstract protected function executeAllocateResource(DrydockLease $lease); + public function canAllocateResourceForLease(DrydockLease $lease) { + return true; + } + abstract protected function executeInitializePendingResource( + DrydockResource $resource, + DrydockLease $lease); + + abstract protected function executeAllocateResource( + DrydockResource $resource, + DrydockLease $lease); + + /** + * Closes a previously allocated resource, performing any desired + * cleanup. + * + * After this method executes, the release status is moved to `CLOSED`. + * + * If release fails, throw an exception. + * + * @param DrydockResource Resource to close. + * @return void + */ + abstract protected function executeCloseResource( + DrydockResource $resource); + + /** + * Return whether or not a resource that now has no leases on it + * should be automatically closed. + * + * @param DrydockAllocationContext Relevant contextual information. + * @param DrydockResource The resource that has no more leases on it. + * @return bool + */ + abstract protected function shouldCloseUnleasedResource( + DrydockAllocationContext $context, + DrydockResource $resource); + + final public function closeResource(DrydockResource $resource) { + $resource->openTransaction(); + $resource->setStatus(DrydockResourceStatus::STATUS_CLOSING); + $resource->save(); + + $statuses = array( + DrydockLeaseStatus::STATUS_PENDING, + DrydockLeaseStatus::STATUS_ACTIVE, + ); + + $leases = id(new DrydockLeaseQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withResourceIDs(array($resource->getID())) + ->withStatuses($statuses) + ->execute(); + + foreach ($leases as $lease) { + switch ($lease->getStatus()) { + case DrydockLeaseStatus::STATUS_PENDING: + $message = pht('Breaking pending lease (resource closing).'); + $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); + break; + case DrydockLeaseStatus::STATUS_ACTIVE: + $message = pht('Releasing active lease (resource closing).'); + $this->releaseLease($resource, $lease, true); + $lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED); + break; + } + self::writeLog($resource, $lease, $message); + $lease->save(); + } + + $this->executeCloseResource($resource); + + $resource->setStatus(DrydockResourceStatus::STATUS_CLOSED); + $resource->save(); + $resource->saveTransaction(); + } + + final public function initializePendingResource( + DrydockResource $resource, + DrydockLease $lease) { + + $scope = $this->pushActiveScope($resource, $lease); + + $this->log(pht( + 'Blueprint \'%s\': Initializing Resource for \'%s\'', + $this->getBlueprintClass(), + $lease->getLeaseName())); + + try { + $this->executeInitializePendingResource($resource, $lease); + } catch (Exception $ex) { + $this->logException($ex); + throw $ex; + } + } + + final public function allocateResource( + DrydockResource $resource, + DrydockLease $lease) { - final public function allocateResource(DrydockLease $lease) { $scope = $this->pushActiveScope(null, $lease); $this->log( @@ -314,7 +456,7 @@ $lease->getLeaseName())); try { - $resource = $this->executeAllocateResource($lease); + $this->executeAllocateResource($resource, $lease); $this->validateAllocatedResource($resource); } catch (Exception $ex) { $this->logException($ex); @@ -389,25 +531,6 @@ return idx(self::getAllBlueprintImplementations(), $class); } - protected function newResourceTemplate($name) { - $resource = id(new DrydockResource()) - ->setBlueprintPHID($this->getInstance()->getPHID()) - ->setBlueprintClass($this->getBlueprintClass()) - ->setType($this->getType()) - ->setStatus(DrydockResourceStatus::STATUS_PENDING) - ->setName($name) - ->save(); - - $this->activeResource = $resource; - - $this->log( - pht( - "Blueprint '%s': Created New Template", - $this->getBlueprintClass())); - - return $resource; - } - /** * Sanity checks that the blueprint is implemented properly. */ @@ -441,7 +564,7 @@ } } - private function pushActiveScope( + public function pushActiveScope( DrydockResource $resource = null, DrydockLease $lease = null) { diff --git a/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php new file mode 100644 --- /dev/null +++ b/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php @@ -0,0 +1,152 @@ +getDetail('max-count'); + + if ($max_count === null) { + $this->log(pht( + 'There is no maximum resource limit specified for this blueprint')); + return true; + } + + $count_pending = 0; + $count_allocating = 0; + $count_open = 0; + + foreach ($pool as $resource) { + switch ($resource->getStatus()) { + case DrydockResourceStatus::STATUS_PENDING: + $count_pending++; + break; + case DrydockResourceStatus::STATUS_ALLOCATING: + $count_allocating++; + break; + case DrydockResourceStatus::STATUS_OPEN: + $count_open++; + break; + default: + $this->log(pht( + 'Resource %d was in the pool of open resources, '. + 'but has non-open status of %d', + $resource->getID(), + $resource->getStatus())); + break; + } + } + + $this->log(pht( + 'There are currently %d pending resources, %d allocating resources '. + 'and %d open resources in the pool.', + $count_pending, + $count_allocating, + $count_open)); + + if (count($pool) < $max_count) { + $this->log(pht( + 'Will permit resource allocation because %d is less than the maximum '. + 'of %d.', + count($pool), + $max_count)); + return true; + } else { + $this->log(pht( + 'Will deny resource allocation because %d is less than the maximum '. + 'of %d.', + count($pool), + $max_count)); + return false; + } + } + + protected function shouldAllocateLease( + DrydockAllocationContext $context, + DrydockResource $resource, + DrydockLease $lease) { + + // If the current resource can allocate a lease, allow it. + if ($context->getCurrentResourceLeaseCount() < + $this->getDetail('leases-per-resource')) { + return true; + } + + // We don't have enough room under the `leases-per-instance` limit, but + // this limit can be bypassed if we've allocated all of the resources + // we allow. + $open_count = $context->getBlueprintOpenResourceCount(); + if ($open_count < $this->getDetail('max-count')) { + if ($this->getDetail('max-count') !== null) { + return false; + } + } + + // Find the resource that has the least leases. + $all_lease_counts_grouped = $context->getResourceLeaseCounts(); + $minimum_lease_count = $all_lease_counts_grouped[$resource->getID()]; + $minimum_lease_resource_id = $resource->getID(); + foreach ($all_lease_counts_grouped as $resource_id => $lease_count) { + if ($minimum_lease_count > $lease_count) { + $minimum_lease_count = $lease_count; + $minimum_lease_resource_id = $resource_id; + } + } + + // If we are that resource, then allow it, otherwise let the other + // less-leased resource run through this logic and allocate the lease. + return $minimum_lease_resource_id === $resource->getID(); + } + + protected function shouldCloseUnleasedResource( + DrydockAllocationContext $context, + DrydockResource $resource) { + + return $context->getBlueprintOpenResourceCount() > + $this->getDetail('min-count'); + } + + public function getFieldSpecifications() { + return array( + 'min-max-header' => array( + 'name' => pht('Allocation Limits'), + 'type' => 'header', + ), + 'min-count' => array( + 'name' => pht('Minimum Resources'), + 'type' => 'int', + 'required' => true, + 'caption' => pht( + 'The minimum number of resources to keep open in '. + 'this pool at all times.'), + ), + 'max-count' => array( + 'name' => pht('Maximum Resources'), + 'type' => 'int', + 'caption' => pht( + 'The maximum number of resources to allow open at any time. '. + 'If the number of resources currently open are equal to '. + '`max-count` and another lease is requested, Drydock will place '. + 'leases on existing resources and thus exceeding '. + '`leases-per-resource`. If this parameter is left blank, then '. + 'this blueprint has no limit on the number of resources it '. + 'can allocate.'), + ), + 'leases-per-resource' => array( + 'name' => pht('Maximum Leases Per Resource'), + 'type' => 'int', + 'required' => true, + 'caption' => pht( + 'The soft limit on the number of leases to allocate to an '. + 'individual resource in the pool. Drydock will choose the '. + 'resource with the lowest number of leases when selecting a '. + 'resource to lease on. If all current resources have '. + '`leases-per-resource` leases on them, then Drydock will allocate '. + 'another resource providing `max-count` would not be exceeded.'. + ' If `max-count` would be exceeded, Drydock will instead '. + 'overallocate the lease to an existing resource and '. + 'exceed the limit specified here.'), + ), + ); + } +} diff --git a/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php new file mode 100644 --- /dev/null +++ b/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php @@ -0,0 +1,100 @@ +getDetail('max-count'); + + if ($max_count === null) { + return parent::canAllocateMoreResources($pool); + } + + $expiry = $this->getDetail('expiry'); + + if ($expiry === null) { + return count($pool) < $max_count; + } + + // Only count resources that haven't yet expired, so we can overallocate + // if another expired resource is about to be closed (but is still waiting + // on it's current resources to be released). + $now = time(); + $pool_copy = array(); + foreach ($pool as $resource) { + $lifetime = $now - $resource->getDateCreated(); + if ($lifetime <= $expiry) { + $pool_copy[] = $resource; + } + } + + return parent::canAllocateMoreResources($pool_copy); + } + + protected function shouldAllocateLease( + DrydockAllocationContext $context, + DrydockResource $resource, + DrydockLease $lease) { + + // If we have no leases allocated to this resource, then we always allow + // the parent logic to evaluate. The reason for this is that an expired + // resource can only be closed when a lease is released, so if the resource + // is open and has no leases, then we'll never reach the code that checks + // the expiry to close it. So we allow this lease to occur, so that we'll + // hit `shouldCloseUnleasedResource` in the future and the resource will + // be closed. + if ($context->getCurrentResourceLeaseCount() === 0) { + return parent::shouldAllocateLease($context, $resource, $lease); + } + + $expiry = $this->getDetail('expiry'); + + if ($expiry !== null) { + $lifetime = time() - $resource->getDateCreated(); + + if ($lifetime > $expiry) { + // Prevent allocation of leases to this resource, since it's over + // it's lifetime allowed. + return false; + } + } + + return parent::shouldAllocateLease($context, $resource, $lease); + } + + protected function shouldCloseUnleasedResource( + DrydockAllocationContext $context, + DrydockResource $resource) { + + $expiry = $this->getDetail('expiry'); + + if ($expiry !== null) { + $lifetime = time() - $resource->getDateCreated(); + + if ($lifetime > $expiry) { + // Force closure of resources that have expired. + return true; + } + } + + return parent::shouldCloseUnleasedResource($context, $resource); + } + + public function getFieldSpecifications() { + return array( + 'expiry-header' => array( + 'name' => pht('Resource Expiration'), + 'type' => 'header', + ), + 'expiry' => array( + 'name' => pht('Expiry Time'), + 'type' => 'int', + 'caption' => pht( + 'After this time (in seconds) has elapsed since resource creation, '. + 'Drydock will no longer lease against the resource, and it will be '. + 'closed when there are no more leases (regardless of minimum '. + 'resource limits).'), + ), + ) + parent::getFieldSpecifications(); + } +} diff --git a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php --- a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php @@ -19,7 +19,14 @@ return false; } - protected function executeAllocateResource(DrydockLease $lease) { + protected function executeInitializePendingResource( + DrydockResource $resource, + DrydockLease $lease) {} + + protected function executeAllocateResource( + DrydockResource $resource, + DrydockLease $lease) { + throw new Exception( pht("Preallocated hosts can't be dynamically allocated.")); } @@ -32,9 +39,9 @@ } protected function shouldAllocateLease( + DrydockAllocationContext $context, DrydockResource $resource, - DrydockLease $lease, - array $other_leases) { + DrydockLease $lease) { return true; } @@ -113,4 +120,20 @@ throw new Exception(pht("No interface of type '%s'.", $type)); } + protected function executeReleaseLease( + DrydockResource $resource, + DrydockLease $lease) { + + // TODO: Remove leased directory + } + + protected function shouldCloseUnleasedResource( + DrydockAllocationContext $context, + DrydockResource $resource) { + + return false; + } + + protected function executeCloseResource(DrydockResource $resource) {} + } 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 @@ -26,14 +26,21 @@ } protected function shouldAllocateLease( + DrydockAllocationContext $context, DrydockResource $resource, - DrydockLease $lease, - array $other_leases) { + DrydockLease $lease) { - return !$other_leases; + return $context->getCurrentResourceLeaseCount() === 0; } - protected function executeAllocateResource(DrydockLease $lease) { + protected function executeInitializePendingResource( + DrydockResource $resource, + DrydockLease $lease) {} + + protected function executeAllocateResource( + DrydockResource $resource, + DrydockLease $lease) { + $repository_id = $lease->getAttribute('repositoryID'); if (!$repository_id) { throw new Exception( @@ -79,15 +86,15 @@ $this->log(pht('Complete.')); - $resource = $this->newResourceTemplate( - pht( + $resource + ->setName(pht( 'Working Copy (%s)', - $repository->getCallsign())); - $resource->setStatus(DrydockResourceStatus::STATUS_OPEN); - $resource->setAttribute('lease.host', $host_lease->getID()); - $resource->setAttribute('path', $path); - $resource->setAttribute('repositoryID', $repository->getID()); - $resource->save(); + $repository->getCallsign())) + ->setStatus(DrydockResourceStatus::STATUS_OPEN) + ->setAttribute('lease.host', $host_lease->getID()) + ->setAttribute('path', $path) + ->setAttribute('repositoryID', $repository->getID()) + ->save(); return $resource; } @@ -117,4 +124,19 @@ throw new Exception(pht("No interface of type '%s'.", $type)); } + protected function executeReleaseLease( + DrydockResource $resource, + DrydockLease $lease) {} + + protected function shouldCloseUnleasedResource( + DrydockAllocationContext $context, + DrydockResource $resource) { + + return false; + } + + protected function executeCloseResource(DrydockResource $resource) { + // TODO: Remove leased directory + } + } diff --git a/src/applications/drydock/constants/DrydockResourceStatus.php b/src/applications/drydock/constants/DrydockResourceStatus.php --- a/src/applications/drydock/constants/DrydockResourceStatus.php +++ b/src/applications/drydock/constants/DrydockResourceStatus.php @@ -7,12 +7,16 @@ const STATUS_CLOSED = 2; const STATUS_BROKEN = 3; const STATUS_DESTROYED = 4; + const STATUS_CLOSING = 5; + const STATUS_ALLOCATING = 6; public static function getNameForStatus($status) { $map = array( self::STATUS_PENDING => pht('Pending'), + self::STATUS_ALLOCATING => pht('Allocating'), self::STATUS_OPEN => pht('Open'), self::STATUS_CLOSED => pht('Closed'), + self::STATUS_CLOSING => pht('Closing'), self::STATUS_BROKEN => pht('Broken'), self::STATUS_DESTROYED => pht('Destroyed'), ); @@ -23,8 +27,10 @@ public static function getAllStatuses() { return array( self::STATUS_PENDING, + self::STATUS_ALLOCATING, self::STATUS_OPEN, self::STATUS_CLOSED, + self::STATUS_CLOSING, self::STATUS_BROKEN, self::STATUS_DESTROYED, ); diff --git a/src/applications/drydock/controller/DrydockBlueprintCreateController.php b/src/applications/drydock/controller/DrydockBlueprintCreateController.php --- a/src/applications/drydock/controller/DrydockBlueprintCreateController.php +++ b/src/applications/drydock/controller/DrydockBlueprintCreateController.php @@ -34,6 +34,11 @@ ->setError($e_blueprint); foreach ($implementations as $implementation_name => $implementation) { + if ($implementation->isTest()) { + // Never show testing blueprints in the interface. + continue; + } + $disabled = !$implementation->isEnabled(); $control->addButton( diff --git a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php --- a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php +++ b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php @@ -19,6 +19,10 @@ 'param' => 'name=value,...', 'help' => pht('Resource specficiation.'), ), + array( + 'name' => 'in-process', + 'help' => pht('Acquire lease in-process.'), + ), )); } @@ -40,7 +44,9 @@ $attributes = $options->parse($attributes); } - PhabricatorWorker::setRunAllTasksInProcess(true); + if ($args->getArg('in-process')) { + PhabricatorWorker::setRunAllTasksInProcess(true); + } $lease = id(new DrydockLease()) ->setResourceType($resource_type); @@ -48,8 +54,18 @@ $lease->setAttributes($attributes); } $lease - ->queueForActivation() - ->waitUntilActive(); + ->queueForActivation(); + + while (true) { + try { + $lease->waitUntilActive(); + break; + } catch (PhabricatorWorkerYieldException $ex) { + $console->writeOut( + "%s\n", + pht('Task yielded while acquiring %s...', $lease->getID())); + } + } $console->writeOut("%s\n", pht('Acquired Lease %s', $lease->getID())); return 0; diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php --- a/src/applications/drydock/storage/DrydockResource.php +++ b/src/applications/drydock/storage/DrydockResource.php @@ -75,36 +75,8 @@ } public function closeResource() { - $this->openTransaction(); - $statuses = array( - DrydockLeaseStatus::STATUS_PENDING, - DrydockLeaseStatus::STATUS_ACTIVE, - ); - - $leases = id(new DrydockLeaseQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withResourceIDs(array($this->getID())) - ->withStatuses($statuses) - ->execute(); - - foreach ($leases as $lease) { - switch ($lease->getStatus()) { - case DrydockLeaseStatus::STATUS_PENDING: - $message = pht('Breaking pending lease (resource closing).'); - $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); - break; - case DrydockLeaseStatus::STATUS_ACTIVE: - $message = pht('Releasing active lease (resource closing).'); - $lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED); - break; - } - DrydockBlueprintImplementation::writeLog($this, $lease, $message); - $lease->save(); - } - - $this->setStatus(DrydockResourceStatus::STATUS_CLOSED); - $this->save(); - $this->saveTransaction(); + $blueprint = $this->getBlueprint(); + $blueprint->closeResource($this); } diff --git a/src/applications/drydock/util/DrydockAllocationContext.php b/src/applications/drydock/util/DrydockAllocationContext.php new file mode 100644 --- /dev/null +++ b/src/applications/drydock/util/DrydockAllocationContext.php @@ -0,0 +1,98 @@ +getTableName(); + $table_resource = id(new DrydockResource())->getTableName(); + $table_lease = id(new DrydockLease())->getTableName(); + + $conn = $blueprint->establishConnection('r'); + + $result = queryfx_one( + $conn, + 'SELECT COUNT(id) AS count '. + 'FROM %T '. + 'WHERE blueprintPHID = %s '. + 'AND status IN (%Ld)', + $table_resource, + $blueprint->getPHID(), + array( + DrydockResourceStatus::STATUS_PENDING, + DrydockResourceStatus::STATUS_OPEN, + DrydockResourceStatus::STATUS_ALLOCATING, + )); + $this->setBlueprintOpenResourceCount($result['count']); + + $results = queryfx_all( + $conn, + 'SELECT '. + ' resource.id AS resourceID, '. + ' COUNT(lease.id) AS leaseCount '. + 'FROM %T AS resource '. + 'LEFT JOIN %T AS lease '. + ' ON lease.resourceID = resource.id '. + 'WHERE resource.blueprintPHID = %s '. + 'AND resource.status IN (%Ld) '. + 'AND lease.status IN (%Ld) '. + 'GROUP BY resource.id', + $table_resource, + $table_lease, + $blueprint->getPHID(), + array( + DrydockResourceStatus::STATUS_PENDING, + DrydockResourceStatus::STATUS_OPEN, + DrydockResourceStatus::STATUS_ALLOCATING, + ), + array( + DrydockLeaseStatus::STATUS_PENDING, + DrydockLeaseStatus::STATUS_ACQUIRING, + DrydockLeaseStatus::STATUS_ACTIVE, + )); + $results = ipull($results, 'leaseCount', 'resourceID'); + $this->setResourceLeaseCounts($results); + + if ($resource !== null) { + $this->setCurrentResourceLeaseCount(idx($results, $resource->getID(), 0)); + } + + // $lease is not yet used, but it's passed in so we can add additional + // contextual statistics later. + } + + public function setBlueprintOpenResourceCount($blueprint_resource_count) { + $this->blueprintOpenResourceCount = $blueprint_resource_count; + return $this; + } + + public function getBlueprintOpenResourceCount() { + return $this->blueprintOpenResourceCount; + } + + public function setResourceLeaseCounts($resource_lease_counts) { + $this->resourceLeaseCounts = $resource_lease_counts; + return $this; + } + + public function getResourceLeaseCounts() { + return $this->resourceLeaseCounts; + } + + public function setCurrentResourceLeaseCount($resource_lease_counts) { + $this->currentResourceLeaseCount = $resource_lease_counts; + return $this; + } + + public function getCurrentResourceLeaseCount() { + return $this->currentResourceLeaseCount; + } + +} diff --git a/src/applications/drydock/view/DrydockResourceListView.php b/src/applications/drydock/view/DrydockResourceListView.php --- a/src/applications/drydock/view/DrydockResourceListView.php +++ b/src/applications/drydock/view/DrydockResourceListView.php @@ -26,6 +26,7 @@ $item->addAttribute($status); switch ($resource->getStatus()) { + case DrydockResourceStatus::STATUS_ALLOCATING: case DrydockResourceStatus::STATUS_PENDING: $item->setStatusIcon('fa-dot-circle-o yellow'); break; 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 @@ -39,10 +39,21 @@ protected function doWork() { $lease = $this->loadLease(); + + if ($lease->getStatus() != DrydockLeaseStatus::STATUS_PENDING) { + // We can't handle non-pending leases. + return; + } + $this->logToDrydock(pht('Allocating Lease')); try { $this->allocateLease($lease); + } catch (PhabricatorWorkerYieldException $ex) { + $this->logToDrydock(pht( + 'Another Drydock lease is being allocated right now; '. + 'lease acquisition will continue soon')); + throw $ex; } catch (Exception $ex) { // TODO: We should really do this when archiving the task, if we've @@ -50,7 +61,9 @@ // and always fail after the first retry right now, so this is // functionally equivalent. $lease->reload(); - if ($lease->getStatus() == DrydockLeaseStatus::STATUS_PENDING) { + if ($lease->getStatus() == DrydockLeaseStatus::STATUS_PENDING || + $lease->getStatus() == DrydockLeaseStatus::STATUS_ACQUIRING) { + $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); $lease->save(); } @@ -76,6 +89,16 @@ $blueprints = $this->loadAllBlueprints(); + $lock = PhabricatorGlobalLock::newLock('drydockallocation'); + try { + $lock->lock(); + } catch (PhutilLockException $ex) { + // The lock is expected to be released reasonably quickly, so + // just push the work to the back of the queue and let it be + // reprocessed as soon as possible. + throw new PhabricatorWorkerYieldException(1); + } + // TODO: Policy stuff. $pool = id(new DrydockResource())->loadAllWhere( 'type = %s AND status = %s', @@ -116,68 +139,226 @@ } if (!$resource) { - $blueprints = DrydockBlueprintImplementation - ::getAllBlueprintImplementationsForResource($type); + // Attempt to use pending resources if we can. + $pool = id(new DrydockResource())->loadAllWhere( + 'type = %s AND status = %s', + $lease->getResourceType(), + DrydockResourceStatus::STATUS_PENDING); $this->logToDrydock( - pht('Found %d Blueprints', count($blueprints))); + pht('Found %d Pending Resource(s)', count($pool))); - foreach ($blueprints as $key => $candidate_blueprint) { - if (!$candidate_blueprint->isEnabled()) { - unset($blueprints[$key]); + $candidates = array(); + foreach ($pool as $key => $candidate) { + if (!isset($blueprints[$candidate->getBlueprintPHID()])) { + unset($pool[$key]); continue; } + + $blueprint = $blueprints[$candidate->getBlueprintPHID()]; + $implementation = $blueprint->getImplementation(); + + if ($implementation->filterResource($candidate, $lease)) { + $candidates[] = $candidate; + } } $this->logToDrydock( - pht('%d Blueprints Enabled', count($blueprints))); + pht('%d Pending Resource(s) Remain', + count($candidates))); + + $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; + } + } + } + } + + if ($resource) { + $lock->unlock(); + } else { + $blueprints = id(new DrydockBlueprintQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->execute(); + $blueprints = mpull($blueprints, 'getImplementation', 'getPHID'); + + $this->logToDrydock( + pht('Found %d Blueprints', count($blueprints))); foreach ($blueprints as $key => $candidate_blueprint) { - if (!$candidate_blueprint->canAllocateMoreResources($pool)) { + if (!$candidate_blueprint->isEnabled()) { + unset($blueprints[$key]); + continue; + } + + if ($candidate_blueprint->getType() !== + $lease->getResourceType()) { unset($blueprints[$key]); continue; } } $this->logToDrydock( - pht('%d Blueprints Can Allocate', count($blueprints))); + pht('%d Blueprints Enabled', count($blueprints))); - if (!$blueprints) { - $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); - $lease->save(); + $resources_per_blueprint = id(new DrydockResourceQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withStatuses(array( + DrydockResourceStatus::STATUS_PENDING, + DrydockResourceStatus::STATUS_OPEN, + DrydockResourceStatus::STATUS_ALLOCATING, + )) + ->execute(); + $resources_per_blueprint = mgroup( + $resources_per_blueprint, + 'getBlueprintPHID'); + + try { + foreach ($blueprints as $key => $candidate_blueprint) { + $scope = $candidate_blueprint->pushActiveScope(null, $lease); + + $rpool = idx($resources_per_blueprint, $key, array()); + if (!$candidate_blueprint->canAllocateMoreResources($rpool)) { + unset($blueprints[$key]); + continue; + } + + if (!$candidate_blueprint->canAllocateResourceForLease($lease)) { + unset($blueprints[$key]); + continue; + } + } $this->logToDrydock( - pht( - "There are no resources of type '%s' available, and no ". - "blueprints which can allocate new ones.", - $type)); + pht('%d Blueprints Can Allocate', count($blueprints))); + + if (!$blueprints) { + $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN); + $lease->save(); + + $this->logToDrydock( + pht( + "There are no resources of type '%s' available, and no ". + "blueprints which can allocate new ones.", + $type)); + + $lock->unlock(); + return; + } - return; + // TODO: Rank intelligently. + shuffle($blueprints); + + $blueprint = head($blueprints); + + // Create and save the resource preemptively with STATUS_ALLOCATING + // before we unlock, so that other workers will correctly count the + // new resource "to be allocated" when determining if they can allocate + // more resources to a blueprint. + $resource = id(new DrydockResource()) + ->setBlueprintPHID($blueprint->getInstance()->getPHID()) + ->setType($blueprint->getType()) + ->setName(pht('Pending Allocation')) + ->setStatus(DrydockResourceStatus::STATUS_ALLOCATING) + ->save(); + + // Pre-emptively allocate the lease on the resource inside the lock, + // to ensure that other allocators don't cause this worker to lose + // an allocation race. If we fail to allocate a lease here, then the + // blueprint is allocating resources it can't lease against. + // + // NOTE; shouldAllocateLease is specified to only check resource + // constraints, which means that it shouldn't be checking compatibility + // of details on resources or leases. If there are any + // shouldAllocateLease that use details on the resources or leases to + // complete their work, then we might have to change this to: + // + // $lease->setStatus(DrydockLeaseStatus::STATUS_ACQUIRING); + // $lease->setResourceID($resource->getID()); + // $lease->attachResource($resource); + // $lease->save(); + // + // and bypass the resource quota logic entirely (and just assume that + // a resource allocated by a blueprint can have the lease allocated + // against it). + // + if (!$blueprint->allocateLease($resource, $lease)) { + throw new Exception( + 'Blueprint allocated a resource, but can\'t lease against it.'); + } + + $this->logToDrydock( + pht('Pre-emptively allocated the lease against the new resource.')); + + // We now have to set the resource into Pending status, now that the + // initial lease has been grabbed on the resource. This ensures that + // as soon as we leave the lock, other allocators can start taking + // leases on it. If we didn't do this, we can run into a scenario + // where all resources are in "ALLOCATING" status when an allocator + // runs, and instead of overleasing, the request would fail. + // + // TODO: I think this means we can remove the "ALLOCATING" status now, + // but I'm not entirely sure. It's only ever used inside the lock, so + // I don't think any other allocators can race when attempting to + // use a still-allocating resource. + $resource + ->setStatus(DrydockResourceStatus::STATUS_PENDING) + ->save(); + + $this->logToDrydock( + pht('Moved the resource to the pending status.')); + + // We must allow some initial set up of resource attributes within the + // lock such that when we exit, method calls to canAllocateLease will + // succeed even for pending resources. + $this->logToDrydock( + pht('Started initialization of pending resource.')); + + $blueprint->initializePendingResource($resource, $lease); + + $this->logToDrydock( + pht('Finished initialization of pending resource.')); + + $lock->unlock(); + } catch (Exception $ex) { + $lock->unlock(); + throw $ex; } - // 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?')); + try { + $blueprint->allocateResource($resource, $lease); + } catch (Exception $ex) { + $resource->setStatus(DrydockResourceStatus::STATUS_BROKEN); + $resource->save(); + throw $ex; } + + // We do not need to call allocateLease here, because we have already + // performed this check inside the lock. If the logic at the end of the + // lock changes to bypass allocateLease, then we probably need to do some + // logic like (where STATUS_RESERVED does not count towards allocation + // limits): + // + // $lock->lock(10000); + // $lease->setStatus(DrydockLeaseStatus::STATUS_RESERVED); + // $lease->save(); + // try { + // if (!$blueprint->allocateLease($resource, $lease)) { + // throw new Exception('Lost an allocation race?'); + // } + // } catch (Exception $ex) { + // $lock->unlock(); + // throw $ex; + // } + // $lock->unlock(); + // } $blueprint = $resource->getBlueprint(); diff --git a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php --- a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php @@ -21,29 +21,49 @@ $settings = $this->getSettings(); - // Create the lease. - $lease = id(new DrydockLease()) - ->setResourceType('host') - ->setOwnerPHID($build_target->getPHID()) - ->setAttributes( + // This build step is reentrant, because waitUntilActive may + // throw PhabricatorWorkerYieldException. Check to see if there + // is already a lease on the build target, and if so, wait until + // that lease is active instead of creating a new one. + $artifacts = id(new HarbormasterBuildArtifactQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withBuildTargetPHIDs(array($build_target->getPHID())) + ->execute(); + $artifact = count($artifacts) > 0 ? head($artifacts) : null; + + if ($artifact === null) { + // Create the lease. + $lease = id(new DrydockLease()) + ->setResourceType('host') + ->setOwnerPHID($build_target->getPHID()) + ->setAttributes( + array( + 'platform' => $settings['platform'], + )) + ->queueForActivation(); + + // Create the associated artifact. + $artifact = $build_target->createArtifact( + PhabricatorUser::getOmnipotentUser(), + $settings['name'], + HarbormasterHostArtifact::ARTIFACTCONST, array( - 'platform' => $settings['platform'], - )) - ->queueForActivation(); + 'drydockLeasePHID' => $lease->getPHID(), + )); + } else { + // Load the lease. + $impl = $artifact->getArtifactImplementation(); + $lease = $impl->loadArtifactLease(PhabricatorUser::getOmnipotentUser()); + } // Wait until the lease is fulfilled. - // TODO: This will throw an exception if the lease can't be fulfilled; - // we should treat that as build failure not build error. - $lease->waitUntilActive(); - - // Create the associated artifact. - $artifact = $build_target->createArtifact( - PhabricatorUser::getOmnipotentUser(), - $settings['name'], - HarbormasterHostArtifact::ARTIFACTCONST, - array( - 'drydockLeasePHID' => $lease->getPHID(), - )); + try { + $lease->waitUntilActive(); + } catch (PhabricatorWorkerYieldException $ex) { + throw $ex; + } catch (Exception $ex) { + throw new HarbormasterBuildFailureException($ex->getMessage()); + } } public function getArtifactOutputs() {