1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-19 21:32:43 +01:00

Reduce garbage-level of Drydock Allocator implementation

Summary:
Ref T9253. The Drydock allocator is very pseudocodey right now. Particularly, it was written before Blueprints were concrete.

Reorganize it to make its responsibilities and error handling behaviors more clear.

In particular, the Allocator does not manage locks. It's primarily trying to reject allocations which can not possibly work. Blueprints are responsible for locks. See some discussion in D10304.

NOTE: This code probably doesn't work as written, see future diffs.

Test Plan: See future diffs.

Reviewers: hach-que, chad

Reviewed By: hach-que, chad

Subscribers: cburroughs

Maniphest Tasks: T9253

Differential Revision: https://secure.phabricator.com/D14114
This commit is contained in:
epriestley 2015-09-21 04:43:25 -07:00
parent 5362d3366c
commit bb28f94f9b
4 changed files with 430 additions and 205 deletions

View file

@ -68,41 +68,26 @@ abstract class DrydockBlueprintImplementation extends Phobject {
/* -( Lease Acquisition )-------------------------------------------------- */
/**
* @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 @@ abstract class DrydockBlueprintImplementation extends Phobject {
/* -( Resource Allocation )------------------------------------------------ */
public function canAllocateMoreResources(array $pool) {
return true;
}
abstract protected function executeAllocateResource(DrydockLease $lease);
/**
* 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);
final public function allocateResource(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);
/**
* 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);
/**
* 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 @@ abstract class DrydockBlueprintImplementation extends Phobject {
$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 @@ abstract class DrydockBlueprintImplementation extends Phobject {
->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);
}

View file

@ -15,9 +15,31 @@ final class DrydockWorkingCopyBlueprintImplementation
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 @@ final class DrydockWorkingCopyBlueprintImplementation
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(

View file

@ -51,16 +51,7 @@ final class DrydockBlueprint extends DrydockDAO
}
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 @@ final class DrydockBlueprint extends DrydockDAO
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 )------------------------- */

View file

@ -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;
}
return $this->lease;
}
$viewer = $this->getViewer();
private function logToDrydock($message) {
DrydockBlueprintImplementation::writeLog(
null,
$this->loadLease(),
$message);
// 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 $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();
}
throw $ex;
}
}
private function loadAllBlueprints() {
$viewer = PhabricatorUser::getOmnipotentUser();
$instances = id(new DrydockBlueprintQuery())
->setViewer($viewer)
->execute();
$blueprints = array();
foreach ($instances as $instance) {
$blueprints[$instance->getPHID()] = $instance;
}
return $blueprints;
$this->allocateLease($lease);
}
private function allocateLease(DrydockLease $lease) {
$type = $lease->getResourceType();
$blueprints = $this->loadBlueprintsForAllocatingLease($lease);
$blueprints = $this->loadAllBlueprints();
// 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()));
}
// TODO: Policy stuff.
$pool = id(new DrydockResource())->loadAllWhere(
'type = %s AND status = %s',
$lease->getResourceType(),
DrydockResourceStatus::STATUS_OPEN);
// First, try to find a suitable open resource which we can acquire a new
// lease on.
$resources = $this->loadResourcesForAllocatingLease($blueprints, $lease);
$this->logToDrydock(
pht('Found %d Open Resource(s)', count($pool)));
// If no resources exist yet, see if we can build one.
if (!$resources) {
$usable_blueprints = $this->removeOverallocatedBlueprints(
$blueprints,
$lease);
$candidates = array();
foreach ($pool as $key => $candidate) {
if (!isset($blueprints[$candidate->getBlueprintPHID()])) {
unset($pool[$key]);
// 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;
}
}
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);
}
}
/**
* Load a list of all resources which a given lease can possibly be
* allocated against.
*
* @param list<DrydockBlueprint> Blueprints which may produce suitable
* resources.
* @param DrydockLease Requested lease.
* @return list<DrydockResource> 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();
$keep = array();
foreach ($resources as $key => $resource) {
if (!$resource->canAllocateLease($lease)) {
continue;
}
$blueprint = $blueprints[$candidate->getBlueprintPHID()];
$implementation = $blueprint->getImplementation();
if ($implementation->filterResource($candidate, $lease)) {
$candidates[] = $candidate;
}
$keep[$key] = $resource;
}
$this->logToDrydock(pht('%d Open Resource(s) Remain', count($candidates)));
return $keep;
}
$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;
}
}
/**
* Rank blueprints by suitability for building a new resource for a
* particular lease.
*
* @param list<DrydockBlueprint> List of blueprints.
* @param DrydockLease Requested lease.
* @return list<DrydockBlueprint> Ranked list of blueprints.
*/
private function rankBlueprints(array $blueprints, DrydockLease $lease) {
assert_instances_of($blueprints, 'DrydockBlueprint');
// TODO: Implement improvements to this ranking algorithm if they become
// available.
shuffle($blueprints);
return $blueprints;
}
/**
* Rank resources by suitability for allocating a particular lease.
*
* @param list<DrydockResource> List of resources.
* @param DrydockLease Requested lease.
* @return list<DrydockResource> Ranked list of resources.
*/
private function rankResources(array $resources, DrydockLease $lease) {
assert_instances_of($resources, 'DrydockResource');
// 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<DrydockBlueprint> List of qualifying blueprints.
*/
private function loadBlueprintsForAllocatingLease(
DrydockLease $lease) {
$viewer = $this->getViewer();
$impls = $this->loadBlueprintImplementationsForAllocatingLease($lease);
if (!$impls) {
return array();
}
if (!$resource) {
$blueprints = DrydockBlueprintImplementation
::getAllBlueprintImplementationsForResource($type);
// TODO: When blueprints can be disabled, this query should ignore disabled
// blueprints.
$this->logToDrydock(
pht('Found %d Blueprints', count($blueprints)));
$blueprints = id(new DrydockBlueprintQuery())
->setViewer($viewer)
->withBlueprintClasses(array_keys($impls))
->execute();
foreach ($blueprints as $key => $candidate_blueprint) {
if (!$candidate_blueprint->isEnabled()) {
unset($blueprints[$key]);
continue;
}
$keep = array();
foreach ($blueprints as $key => $blueprint) {
if (!$blueprint->canEverAllocateResourceForLease($lease)) {
continue;
}
$this->logToDrydock(
pht('%d Blueprints Enabled', count($blueprints)));
foreach ($blueprints as $key => $candidate_blueprint) {
if (!$candidate_blueprint->canAllocateMoreResources($pool)) {
unset($blueprints[$key]);
continue;
}
}
$this->logToDrydock(
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));
return;
}
// 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;
}
/**
* 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<DrydockBlueprintImplementation> 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;
}
// Don't use blueprint types which can't allocate the correct kind of
// resource.
if ($impl->getType() != $lease->getResourceType()) {
continue;
}
if (!$impl->canAnyBlueprintEverAllocateResourceForLease($lease)) {
continue;
}
$keep[$key] = $impl;
}
return $keep;
}
/**
* Remove blueprints which are too heavily allocated to build a resource for
* a lease from a list of blueprints.
*
* @param list<DrydockBlueprint> List of blueprints.
* @param list<DrydockBlueprint> 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;
}
$keep[$key] = $blueprint;
}
return $keep;
}
}