diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php --- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php @@ -45,6 +45,16 @@ return true; } + public function shouldAllocateSupplementalResource( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + // We want to use every host in an Almanac service, since the amount of + // hardware is fixed and there's normally no value in packing leases onto a + // subset of it. Always build a new supplemental resource if we can. + return true; + } + public function canAllocateResourceForLease( DrydockBlueprint $blueprint, DrydockLease $lease) { 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 @@ -139,6 +139,38 @@ DrydockResource $resource, DrydockLease $lease); + /** + * Return true to try to allocate a new resource and expand the resource + * pool instead of permitting an otherwise valid acquisition on an existing + * resource. + * + * This allows the blueprint to provide a soft hint about when the resource + * pool should grow. + * + * Returning "true" in all cases generally makes sense when a blueprint + * controls a fixed pool of resources, like a particular number of physical + * hosts: you want to put all the hosts in service, so whenever it is + * possible to allocate a new host you want to do this. + * + * Returning "false" in all cases generally make sense when a blueprint + * has a flexible pool of expensive resources and you want to pack leases + * onto them as tightly as possible. + * + * @param DrydockBlueprint The blueprint for an existing resource being + * acquired. + * @param DrydockResource The resource being acquired, which we may want to + * build a supplemental resource for. + * @param DrydockLease The current lease performing acquisition. + * @return bool True to prefer allocating a supplemental resource. + * + * @task lease + */ + public function shouldAllocateSupplementalResource( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + return false; + } /* -( Resource Allocation )------------------------------------------------ */ 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 @@ -278,6 +278,15 @@ return $interface; } + public function shouldAllocateSupplementalResource( + DrydockResource $resource, + DrydockLease $lease) { + return $this->getImplementation()->shouldAllocateSupplementalResource( + $this, + $resource, + $lease); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php --- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php @@ -306,6 +306,7 @@ $allocated = false; foreach ($resources as $resource) { try { + $resource = $this->newResourceForAcquisition($resource, $lease); $this->acquireLease($resource, $lease); $allocated = true; break; @@ -319,6 +320,10 @@ // got around to acquiring it, we just got unlucky. We can yield and // try again later. $yields[] = $ex; + } catch (PhabricatorWorkerYieldException $ex) { + // We can be told to yield, particularly by the supplemental allocator + // trying to give us a supplemental resource. + $yields[] = $ex; } catch (Exception $ex) { $exceptions[] = $ex; } @@ -791,6 +796,73 @@ } } + private function newResourceForAcquisition( + DrydockResource $resource, + DrydockLease $lease) { + + // If the resource has no leases against it, never build a new one. This is + // likely already a new resource that just activated. + $viewer = $this->getViewer(); + + $statuses = array( + DrydockLeaseStatus::STATUS_PENDING, + DrydockLeaseStatus::STATUS_ACQUIRED, + DrydockLeaseStatus::STATUS_ACTIVE, + ); + + $leases = id(new DrydockLeaseQuery()) + ->setViewer($viewer) + ->withResourcePHIDs(array($resource->getPHID())) + ->withStatuses($statuses) + ->setLimit(1) + ->execute(); + if (!$leases) { + return $resource; + } + + // If we're about to get a lease on a resource, check if the blueprint + // wants to allocate a supplemental resource. If it does, try to perform a + // new allocation instead. + $blueprint = $resource->getBlueprint(); + if (!$blueprint->shouldAllocateSupplementalResource($resource, $lease)) { + return $resource; + } + + // If the blueprint is already overallocated, we can't allocate a new + // resource. Just return the existing resource. + $remaining = $this->removeOverallocatedBlueprints( + array($blueprint), + $lease); + if (!$remaining) { + return $resource; + } + + // Try to build a new resource. + try { + $new_resource = $this->allocateResource($blueprint, $lease); + } catch (Exception $ex) { + $blueprint->logEvent( + DrydockResourceAllocationFailureLogType::LOGCONST, + array( + 'class' => get_class($ex), + 'message' => $ex->getMessage(), + )); + + return $resource; + } + + // If we can't actually acquire the new resource yet, just yield. + // (We could try to move forward with the original resource instead.) + $acquirable = $this->removeUnacquirableResources( + array($new_resource), + $lease); + if (!$acquirable) { + throw new PhabricatorWorkerYieldException(15); + } + + return $new_resource; + } + /* -( Activating Leases )-------------------------------------------------- */