D14127.diff
No OneTemporary
Actions

Size

34 KB

Referenced Files

None

Subscribers

None

D14127.diff
View Options

	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
	@@ -838,6 +838,7 @@
	'DrydockLeaseSearchEngine' => 'applications/drydock/query/DrydockLeaseSearchEngine.php',
	'DrydockLeaseStatus' => 'applications/drydock/constants/DrydockLeaseStatus.php',
	'DrydockLeaseViewController' => 'applications/drydock/controller/DrydockLeaseViewController.php',
	+ 'DrydockLeaseWorker' => 'applications/drydock/worker/DrydockLeaseWorker.php',
	'DrydockLog' => 'applications/drydock/storage/DrydockLog.php',
	'DrydockLogController' => 'applications/drydock/controller/DrydockLogController.php',
	'DrydockLogListController' => 'applications/drydock/controller/DrydockLogListController.php',
	@@ -861,10 +862,13 @@
	'DrydockResourceSearchEngine' => 'applications/drydock/query/DrydockResourceSearchEngine.php',
	'DrydockResourceStatus' => 'applications/drydock/constants/DrydockResourceStatus.php',
	'DrydockResourceViewController' => 'applications/drydock/controller/DrydockResourceViewController.php',
	+ 'DrydockResourceWorker' => 'applications/drydock/worker/DrydockResourceWorker.php',
	'DrydockSFTPFilesystemInterface' => 'applications/drydock/interface/filesystem/DrydockSFTPFilesystemInterface.php',
	'DrydockSSHCommandInterface' => 'applications/drydock/interface/command/DrydockSSHCommandInterface.php',
	'DrydockSlotLock' => 'applications/drydock/storage/DrydockSlotLock.php',
	+ 'DrydockSlotLockException' => 'applications/drydock/exception/DrydockSlotLockException.php',
	'DrydockWebrootInterface' => 'applications/drydock/interface/webroot/DrydockWebrootInterface.php',
	+ 'DrydockWorker' => 'applications/drydock/worker/DrydockWorker.php',
	'DrydockWorkingCopyBlueprintImplementation' => 'applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php',
	'FeedConduitAPIMethod' => 'applications/feed/conduit/FeedConduitAPIMethod.php',
	'FeedPublishConduitAPIMethod' => 'applications/feed/conduit/FeedPublishConduitAPIMethod.php',
	@@ -4502,7 +4506,7 @@
	'DoorkeeperSchemaSpec' => 'PhabricatorConfigSchemaSpec',
	'DoorkeeperTagView' => 'AphrontView',
	'DoorkeeperTagsController' => 'PhabricatorController',
	- 'DrydockAllocatorWorker' => 'PhabricatorWorker',
	+ 'DrydockAllocatorWorker' => 'DrydockWorker',
	'DrydockAlmanacServiceHostBlueprintImplementation' => 'DrydockBlueprintImplementation',
	'DrydockApacheWebrootInterface' => 'DrydockWebrootInterface',
	'DrydockBlueprint' => array(
	@@ -4555,6 +4559,7 @@
	'DrydockLeaseSearchEngine' => 'PhabricatorApplicationSearchEngine',
	'DrydockLeaseStatus' => 'DrydockConstants',
	'DrydockLeaseViewController' => 'DrydockLeaseController',
	+ 'DrydockLeaseWorker' => 'DrydockWorker',
	'DrydockLog' => array(
	'DrydockDAO',
	'PhabricatorPolicyInterface',
	@@ -4584,10 +4589,13 @@
	'DrydockResourceSearchEngine' => 'PhabricatorApplicationSearchEngine',
	'DrydockResourceStatus' => 'DrydockConstants',
	'DrydockResourceViewController' => 'DrydockResourceController',
	+ 'DrydockResourceWorker' => 'DrydockWorker',
	'DrydockSFTPFilesystemInterface' => 'DrydockFilesystemInterface',
	'DrydockSSHCommandInterface' => 'DrydockCommandInterface',
	'DrydockSlotLock' => 'DrydockDAO',
	+ 'DrydockSlotLockException' => 'Exception',
	'DrydockWebrootInterface' => 'DrydockInterface',
	+ 'DrydockWorker' => 'PhabricatorWorker',
	'DrydockWorkingCopyBlueprintImplementation' => 'DrydockBlueprintImplementation',
	'FeedConduitAPIMethod' => 'ConduitAPIMethod',
	'FeedPublishConduitAPIMethod' => 'FeedConduitAPIMethod',
	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
	@@ -76,7 +76,7 @@
	->needSlotLock("almanac.host.binding({$binding_phid})");

	try {
	- return $resource->allocateResource(DrydockResourceStatus::STATUS_OPEN);
	+ return $resource->allocateResource();
	} catch (Exception $ex) {
	$exceptions[] = $ex;
	}
	@@ -92,11 +92,9 @@
	DrydockResource $resource,
	DrydockLease $lease) {

	- // TODO: The current rule is one lease per resource, and there's no way to
	- // make that cheaper here than by just trying to acquire the lease below,
	- // so don't do any special checks for now. When we eventually permit
	- // multiple leases per host, we'll need to load leases anyway, so we can
	- // reject fully leased hosts cheaply here.
	+ if (!DrydockSlotLock::isLockFree($this->getLeaseSlotLock($resource))) {
	+ return false;
	+ }

	return true;
	}
	@@ -106,14 +104,17 @@
	DrydockResource $resource,
	DrydockLease $lease) {

	- $resource_phid = $resource->getPHID();
	-
	$lease
	->setActivateWhenAcquired(true)
	- ->needSlotLock("almanac.host.lease({$resource_phid})")
	+ ->needSlotLock($this->getLeaseSlotLock($resource))
	->acquireOnResource($resource);
	}

	+ private function getLeaseSlotLock(DrydockResource $resource) {
	+ $resource_phid = $resource->getPHID();
	+ return "almanac.host.lease({$resource_phid})";
	+ }
	+
	public function getType() {
	return 'host';
	}
	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
	@@ -67,6 +67,12 @@
	DrydockResource $resource,
	DrydockLease $lease);

	+ public function activateLease(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource,
	+ DrydockLease $lease) {
	+ throw new PhutilMethodNotImplementedException();
	+ }

	final public function releaseLease(
	DrydockBlueprint $blueprint,
	@@ -198,6 +204,11 @@
	DrydockBlueprint $blueprint,
	DrydockLease $lease);

	+ public function activateResource(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource) {
	+ throw new PhutilMethodNotImplementedException();
	+ }

	/* -( Resource Interfaces )------------------------------------------------ */

	@@ -276,6 +287,9 @@
	->setStatus(DrydockResourceStatus::STATUS_PENDING)
	->setName($name);

	+ // Pre-allocate the resource PHID.
	+ $resource->setPHID($resource->generatePHID());
	+
	$this->activeResource = $resource;

	$this->log(
	@@ -286,6 +300,25 @@
	return $resource;
	}

	+ protected function newLease(DrydockBlueprint $blueprint) {
	+ return id(new DrydockLease());
	+ }
	+
	+ protected function requireActiveLease(DrydockLease $lease) {
	+ $lease_status = $lease->getStatus();
	+
	+ switch ($lease_status) {
	+ case DrydockLeaseStatus::STATUS_ACQUIRED:
	+ // TODO: Temporary failure.
	+ throw new Exception(pht('Lease still activating.'));
	+ case DrydockLeaseStatus::STATUS_ACTIVE:
	+ return;
	+ default:
	+ // TODO: Permanent failure.
	+ throw new Exception(pht('Lease in bad state.'));
	+ }
	+ }
	+
	private function pushActiveScope(
	DrydockResource $resource = null,
	DrydockLease $lease = null) {
	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
	@@ -17,21 +17,18 @@

	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;
	}

	@@ -39,82 +36,130 @@
	DrydockBlueprint $blueprint,
	DrydockResource $resource,
	DrydockLease $lease) {
	- // TODO: These checks are out of date.

	- $resource_repo = $resource->getAttribute('repositoryID');
	- $lease_repo = $lease->getAttribute('repositoryID');
	+ $have_phid = $resource->getAttribute('repositoryPHID');
	+ $need_phid = $lease->getAttribute('repositoryPHID');

	- return ($resource_repo && $lease_repo && ($resource_repo == $lease_repo));
	- }
	-
	- public function allocateResource(
	- DrydockBlueprint $blueprint,
	- DrydockLease $lease) {
	-
	- $repository_id = $lease->getAttribute('repositoryID');
	- if (!$repository_id) {
	- throw new Exception(
	- pht(
	- "Lease is missing required '%s' attribute.",
	- 'repositoryID'));
	+ if ($need_phid !== $have_phid) {
	+ return false;
	}

	- $repository = id(new PhabricatorRepositoryQuery())
	- ->setViewer(PhabricatorUser::getOmnipotentUser())
	- ->withIDs(array($repository_id))
	- ->executeOne();
	-
	- if (!$repository) {
	- throw new Exception(
	- pht(
	- "Repository '%s' does not exist!",
	- $repository_id));
	+ if (!DrydockSlotLock::isLockFree($this->getLeaseSlotLock($resource))) {
	+ return false;
	}

	- switch ($repository->getVersionControlSystem()) {
	- case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
	- break;
	- default:
	- throw new Exception(pht('Unsupported VCS!'));
	- }
	+ return true;
	+ }

	- // TODO: Policy stuff here too.
	- $host_lease = id(new DrydockLease())
	- ->setResourceType('host')
	- ->waitUntilActive();
	+ public function acquireLease(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource,
	+ DrydockLease $lease) {

	- $path = $host_lease->getAttribute('path').$repository->getCallsign();
	+ $lease
	+ ->needSlotLock($this->getLeaseSlotLock($resource))
	+ ->acquireOnResource($resource);
	+ }

	- $this->log(
	- pht('Cloning %s into %s....', $repository->getCallsign(), $path));
	+ private function getLeaseSlotLock(DrydockResource $resource) {
	+ $resource_phid = $resource->getPHID();
	+ return "workingcopy.lease({$resource_phid})";
	+ }

	- $cmd = $host_lease->getInterface('command');
	- $cmd->execx(
	- 'git clone --origin origin %P %s',
	- $repository->getRemoteURIEnvelope(),
	- $path);
	+ public function allocateResource(
	+ DrydockBlueprint $blueprint,
	+ DrydockLease $lease) {

	- $this->log(pht('Complete.'));
	+ $repository_phid = $lease->getAttribute('repositoryPHID');
	+ $repository = $this->loadRepository($repository_phid);

	$resource = $this->newResourceTemplate(
	$blueprint,
	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();

	- return $resource;
	+ $resource_phid = $resource->getPHID();
	+
	+ $host_lease = $this->newLease($blueprint)
	+ ->setResourceType('host')
	+ ->setOwnerPHID($resource_phid)
	+ ->setAttribute('workingcopy.resourcePHID', $resource_phid)
	+ ->queueForActivation();
	+
	+ // TODO: Add some limits to the number of working copies we can have at
	+ // once?
	+
	+ return $resource
	+ ->setAttribute('repositoryPHID', $repository->getPHID())
	+ ->setAttribute('host.leasePHID', $host_lease->getPHID())
	+ ->allocateResource();
	}

	- public function acquireLease(
	+ public function activateResource(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource) {
	+
	+ $lease = $this->loadHostLease($resource);
	+ $this->requireActiveLease($lease);
	+
	+ $repository_phid = $resource->getAttribute('repositoryPHID');
	+ $repository = $this->loadRepository($repository_phid);
	+ $repository_id = $repository->getID();
	+
	+ $command_type = DrydockCommandInterface::INTERFACE_TYPE;
	+ $interface = $lease->getInterface($command_type);
	+
	+ // TODO: Make this configurable.
	+ $resource_id = $resource->getID();
	+ $root = "/var/drydock/workingcopy-{$resource_id}";
	+ $path = "{$root}/repo/{$repository_id}/";
	+
	+ $interface->execx(
	+ 'git clone -- %s %s',
	+ (string)$repository->getCloneURIObject(),
	+ $path);
	+
	+ $resource
	+ ->setAttribute('workingcopy.root', $root)
	+ ->setAttribute('workingcopy.path', $path)
	+ ->activateResource();
	+ }
	+
	+ public function activateLease(
	DrydockBlueprint $blueprint,
	DrydockResource $resource,
	DrydockLease $lease) {
	- return;
	+
	+ $command_type = DrydockCommandInterface::INTERFACE_TYPE;
	+ $interface = $lease->getInterface($command_type);
	+
	+ $cmd = array();
	+ $arg = array();
	+
	+ $cmd[] = 'git clean -d --force';
	+ $cmd[] = 'git reset --hard HEAD';
	+ $cmd[] = 'git fetch';
	+
	+ $commit = $lease->getAttribute('commit');
	+ $branch = $lease->getAttribute('branch');
	+
	+ if ($commit !== null) {
	+ $cmd[] = 'git reset --hard %s';
	+ $arg[] = $commit;
	+ } else if ($branch !== null) {
	+ $cmd[] = 'git reset --hard %s';
	+ $arg[] = $branch;
	+ }
	+
	+ $cmd = implode(' && ', $cmd);
	+ $argv = array_merge(array($cmd), $arg);
	+
	+ $result = call_user_func_array(
	+ array($interface, 'execx'),
	+ $argv);
	+
	+ $lease->activateOnResource($resource);
	}

	public function getType() {
	@@ -126,7 +171,59 @@
	DrydockResource $resource,
	DrydockLease $lease,
	$type) {
	- // TODO: This blueprint doesn't work at all.
	+
	+ switch ($type) {
	+ case DrydockCommandInterface::INTERFACE_TYPE:
	+ $host_lease = $this->loadHostLease($resource);
	+ $command_interface = $host_lease->getInterface($type);
	+
	+ $path = $resource->getAttribute('workingcopy.path');
	+ $command_interface->setWorkingDirectory($path);
	+
	+ return $command_interface;
	+ }
	}

	+ private function loadRepository($repository_phid) {
	+ $repository = id(new PhabricatorRepositoryQuery())
	+ ->setViewer(PhabricatorUser::getOmnipotentUser())
	+ ->withPHIDs(array($repository_phid))
	+ ->executeOne();
	+ if (!$repository) {
	+ // TODO: Permanent failure.
	+ throw new Exception(
	+ pht(
	+ 'Repository PHID "%s" does not exist.',
	+ $repository_phid));
	+ }
	+
	+ switch ($repository->getVersionControlSystem()) {
	+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
	+ break;
	+ default:
	+ // TODO: Permanent failure.
	+ throw new Exception(pht('Unsupported VCS!'));
	+ }
	+
	+ return $repository;
	+ }
	+
	+ private function loadHostLease(DrydockResource $resource) {
	+ $viewer = PhabricatorUser::getOmnipotentUser();
	+
	+ $lease_phid = $resource->getAttribute('host.leasePHID');
	+
	+ $lease = id(new DrydockLeaseQuery())
	+ ->setViewer($viewer)
	+ ->withPHIDs(array($lease_phid))
	+ ->executeOne();
	+ if (!$lease) {
	+ // TODO: Permanent failure.
	+ throw new Exception(pht('Unable to load lease "%s".', $lease_phid));
	+ }
	+
	+ return $lease;
	+ }
	+
	+
	}
	diff --git a/src/applications/drydock/exception/DrydockSlotLockException.php b/src/applications/drydock/exception/DrydockSlotLockException.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/drydock/exception/DrydockSlotLockException.php
	@@ -0,0 +1,25 @@
	+<?php
	+
	+final class DrydockSlotLockException extends Exception {
	+
	+ private $lockMap;
	+
	+ public function __construct(array $locks) {
	+ $this->lockMap = $locks;
	+
	+ if ($locks) {
	+ $lock_list = array();
	+ foreach ($locks as $lock => $owner_phid) {
	+ $lock_list[] = pht('"%s" (owned by "%s")', $lock, $owner_phid);
	+ }
	+ $message = pht(
	+ 'Unable to acquire slot locks: %s.',
	+ implode(', ', $lock_list));
	+ } else {
	+ $message = pht('Unable to acquire slot locks.');
	+ }
	+
	+ parent::__construct($message);
	+ }
	+
	+}
	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
	@@ -40,8 +40,6 @@
	$attributes = $options->parse($attributes);
	}

	- PhabricatorWorker::setRunAllTasksInProcess(true);
	-
	$lease = id(new DrydockLease())
	->setResourceType($resource_type);
	if ($attributes) {
	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
	@@ -134,6 +134,15 @@
	}


	+ /**
	+ * @task resource
	+ */
	+ public function activateResource(DrydockResource $resource) {
	+ return $this->getImplementation()->activateResource(
	+ $this,
	+ $resource);
	+ }
	+
	/* -( Acquiring Leases )--------------------------------------------------- */


	@@ -166,6 +175,19 @@
	/**
	* @task lease
	*/
	+ public function activateLease(
	+ DrydockResource $resource,
	+ DrydockLease $lease) {
	+ return $this->getImplementation()->activateLease(
	+ $this,
	+ $resource,
	+ $lease);
	+ }
	+
	+
	+ /**
	+ * @task lease
	+ */
	public function releaseLease(
	DrydockResource $resource,
	DrydockLease $lease) {
	diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php
	--- a/src/applications/drydock/storage/DrydockLease.php
	+++ b/src/applications/drydock/storage/DrydockLease.php
	@@ -14,6 +14,7 @@
	private $resource = self::ATTACHABLE;
	private $releaseOnDestruction;
	private $isAcquired = false;
	+ private $isActivated = false;
	private $activateWhenAcquired = false;
	private $slotLocks = array();

	@@ -111,7 +112,12 @@

	$task = PhabricatorWorker::scheduleTask(
	'DrydockAllocatorWorker',
	- $this->getID());
	+ array(
	+ 'leasePHID' => $this->getPHID(),
	+ ),
	+ array(
	+ 'objectPHID' => $this->getPHID(),
	+ ));

	// NOTE: Scheduling the task might execute it in-process, if we're running
	// from a CLI script. Reload the lease to make sure we have the most
	@@ -229,11 +235,11 @@
	if ($this->activateWhenAcquired) {
	$new_status = DrydockLeaseStatus::STATUS_ACTIVE;
	} else {
	- $new_status = DrydockLeaseStatus::STATUS_PENDING;
	+ $new_status = DrydockLeaseStatus::STATUS_ACQUIRED;
	}

	- if ($new_status === DrydockLeaseStatus::STATUS_ACTIVE) {
	- if ($resource->getStatus() === DrydockResourceStatus::STATUS_PENDING) {
	+ if ($new_status == DrydockLeaseStatus::STATUS_ACTIVE) {
	+ if ($resource->getStatus() == DrydockResourceStatus::STATUS_PENDING) {
	throw new Exception(
	pht(
	'Trying to acquire an active lease on a pending resource. '.
	@@ -263,6 +269,45 @@
	return $this->isAcquired;
	}

	+ public function activateOnResource(DrydockResource $resource) {
	+ $expect_status = DrydockLeaseStatus::STATUS_ACQUIRED;
	+ $actual_status = $this->getStatus();
	+ if ($actual_status != $expect_status) {
	+ throw new Exception(
	+ pht(
	+ 'Trying to activate a lease which has the wrong status: status '.
	+ 'must be "%s", actually "%s".',
	+ $expect_status,
	+ $actual_status));
	+ }
	+
	+ if ($resource->getStatus() == DrydockResourceStatus::STATUS_PENDING) {
	+ // TODO: Be stricter about this?
	+ throw new Exception(
	+ pht(
	+ 'Trying to activate a lease on a pending resource.'));
	+ }
	+
	+ $this->openTransaction();
	+
	+ $this
	+ ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE)
	+ ->save();
	+
	+ DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
	+ $this->slotLocks = array();
	+
	+ $this->saveTransaction();
	+
	+ $this->isActivated = true;
	+
	+ return $this;
	+ }
	+
	+ public function isActivatedLease() {
	+ return $this->isActivated;
	+ }
	+

	/* -( PhabricatorPolicyInterface )----------------------------------------- */

	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
	@@ -16,6 +16,7 @@

	private $blueprint = self::ATTACHABLE;
	private $isAllocated = false;
	+ private $isActivated = false;
	private $activateWhenAllocated = false;
	private $slotLocks = array();

	@@ -86,7 +87,7 @@
	return $this;
	}

	- public function allocateResource($status) {
	+ public function allocateResource() {
	if ($this->getID()) {
	throw new Exception(
	pht(
	@@ -131,6 +132,44 @@
	return $this->isAllocated;
	}

	+ public function activateResource() {
	+ if (!$this->getID()) {
	+ throw new Exception(
	+ pht(
	+ 'Trying to activate a resource which has not yet been persisted.'));
	+ }
	+
	+ $expect_status = DrydockResourceStatus::STATUS_PENDING;
	+ $actual_status = $this->getStatus();
	+ if ($actual_status != $expect_status) {
	+ throw new Exception(
	+ pht(
	+ 'Trying to activate a resource from the wrong status. Status must '.
	+ 'be "%s", actually "%s".',
	+ $expect_status,
	+ $actual_status));
	+ }
	+
	+ $this->openTransaction();
	+
	+ $this
	+ ->setStatus(DrydockResourceStatus::STATUS_OPEN)
	+ ->save();
	+
	+ DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
	+ $this->slotLocks = array();
	+
	+ $this->saveTransaction();
	+
	+ $this->isActivated = true;
	+
	+ return $this;
	+ }
	+
	+ public function isActivatedResource() {
	+ return $this->isActivated;
	+ }
	+
	public function closeResource() {

	// TODO: This is super broken and will race other lease writers!
	diff --git a/src/applications/drydock/storage/DrydockSlotLock.php b/src/applications/drydock/storage/DrydockSlotLock.php
	--- a/src/applications/drydock/storage/DrydockSlotLock.php
	+++ b/src/applications/drydock/storage/DrydockSlotLock.php
	@@ -8,6 +8,7 @@
	* machine. These optimistic "slot locks" provide a flexible way to do this
	* sort of simple locking.
	*
	+ * @task info Getting Lock Information
	* @task lock Acquiring and Releasing Locks
	*/
	final class DrydockSlotLock extends DrydockDAO {
	@@ -35,6 +36,17 @@
	) + parent::getConfiguration();
	}

	+
	+/* -( Getting Lock Information )------------------------------------------- */
	+
	+
	+ /**
	+ * Load all locks held by a particular owner.
	+ *
	+ * @param phid Owner PHID.
	+ * @return list<DrydockSlotLock> All held locks.
	+ * @task info
	+ */
	public static function loadLocks($owner_phid) {
	return id(new DrydockSlotLock())->loadAllWhere(
	'ownerPHID = %s',
	@@ -42,6 +54,57 @@
	}


	+ /**
	+ * Test if a lock is currently free.
	+ *
	+ * @param string Lock key to test.
	+ * @return bool True if the lock is currently free.
	+ * @task info
	+ */
	+ public static function isLockFree($lock) {
	+ return self::areLocksFree(array($lock));
	+ }
	+
	+
	+ /**
	+ * Test if a list of locks are all currently free.
	+ *
	+ * @param list<string> List of lock keys to test.
	+ * @return bool True if all locks are currently free.
	+ * @task info
	+ */
	+ public static function areLocksFree(array $locks) {
	+ $lock_map = self::loadHeldLocks($locks);
	+ return !$lock_map;
	+ }
	+
	+
	+ /**
	+ * Load named locks.
	+ *
	+ * @param list<string> List of lock keys to load.
	+ * @return list<DrydockSlotLock> List of held locks.
	+ * @task info
	+ */
	+ public static function loadHeldLocks(array $locks) {
	+ if (!$locks) {
	+ return array();
	+ }
	+
	+ $table = new DrydockSlotLock();
	+ $conn_r = $table->establishConnection('r');
	+
	+ $indexes = array();
	+ foreach ($locks as $lock) {
	+ $indexes[] = PhabricatorHash::digestForIndex($lock);
	+ }
	+
	+ return id(new DrydockSlotLock())->loadAllWhere(
	+ 'lockIndex IN (%Ls)',
	+ $indexes);
	+ }
	+
	+
	/* -( Acquiring and Releasing Locks )-------------------------------------- */


	@@ -74,15 +137,20 @@
	$lock);
	}

	- // TODO: These exceptions are pretty tricky to read. It would be good to
	- // figure out which locks could not be acquired and try to improve the
	- // exception to make debugging easier.
	-
	- queryfx(
	- $conn_w,
	- 'INSERT INTO %T (ownerPHID, lockIndex, lockKey) VALUES %Q',
	- $table->getTableName(),
	- implode(', ', $sql));
	+ try {
	+ queryfx(
	+ $conn_w,
	+ 'INSERT INTO %T (ownerPHID, lockIndex, lockKey) VALUES %Q',
	+ $table->getTableName(),
	+ implode(', ', $sql));
	+ } catch (AphrontDuplicateKeyQueryException $ex) {
	+ // Try to improve the readability of the exception. We might miss on
	+ // this query if the lock has already been released, but most of the
	+ // time we should be able to figure out which locks are already held.
	+ $held = self::loadHeldLocks($locks);
	+ $held = mpull($held, 'getOwnerPHID', 'getLockKey');
	+ throw new DrydockSlotLockException($held);
	+ }
	}


	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
	@@ -5,33 +5,12 @@
	* @task resource Managing Resources
	* @task lease Managing Leases
	*/
	-final class DrydockAllocatorWorker extends PhabricatorWorker {
	-
	- private function getViewer() {
	- return PhabricatorUser::getOmnipotentUser();
	- }
	-
	- private function loadLease() {
	- $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 $lease;
	- }
	+final class DrydockAllocatorWorker extends DrydockWorker {

	protected function doWork() {
	- $lease = $this->loadLease();
	+ $lease_phid = $this->getTaskDataValue('leasePHID');
	+ $lease = $this->loadLease($lease_phid);
	+
	$this->allocateAndAcquireLease($lease);
	}

	@@ -351,6 +330,20 @@
	DrydockLease $lease) {
	$resource = $blueprint->allocateResource($lease);
	$this->validateAllocatedResource($blueprint, $resource, $lease);
	+
	+ // If this resource was allocated as a pending resource, queue a task to
	+ // activate it.
	+ if ($resource->getStatus() == DrydockResourceStatus::STATUS_PENDING) {
	+ PhabricatorWorker::scheduleTask(
	+ 'DrydockResourceWorker',
	+ array(
	+ 'resourcePHID' => $resource->getPHID(),
	+ ),
	+ array(
	+ 'objectPHID' => $resource->getPHID(),
	+ ));
	+ }
	+
	return $resource;
	}

	@@ -429,6 +422,19 @@
	$blueprint->acquireLease($resource, $lease);

	$this->validateAcquiredLease($blueprint, $resource, $lease);
	+
	+ // If this lease has been acquired but not activated, queue a task to
	+ // activate it.
	+ if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACQUIRED) {
	+ PhabricatorWorker::scheduleTask(
	+ 'DrydockLeaseWorker',
	+ array(
	+ 'leasePHID' => $lease->getPHID(),
	+ ),
	+ array(
	+ 'objectPHID' => $lease->getPHID(),
	+ ));
	+ }
	}


	diff --git a/src/applications/drydock/worker/DrydockLeaseWorker.php b/src/applications/drydock/worker/DrydockLeaseWorker.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/drydock/worker/DrydockLeaseWorker.php
	@@ -0,0 +1,81 @@
	+<?php
	+
	+final class DrydockLeaseWorker extends DrydockWorker {
	+
	+ protected function doWork() {
	+ $lease_phid = $this->getTaskDataValue('leasePHID');
	+ $lease = $this->loadLease($lease_phid);
	+
	+ $this->activateLease($lease);
	+ }
	+
	+
	+ private function activateLease(DrydockLease $lease) {
	+ $actual_status = $lease->getStatus();
	+
	+ if ($actual_status != DrydockLeaseStatus::STATUS_ACQUIRED) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht(
	+ 'Trying to activate lease from wrong status ("%s").',
	+ $actual_status));
	+ }
	+
	+ $resource_id = $lease->getResourceID();
	+
	+ $resource = id(new DrydockResourceQuery())
	+ ->setViewer($this->getViewer())
	+ ->withIDs(array($resource_id))
	+ ->executeOne();
	+ if (!$resource) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht(
	+ 'Trying to activate lease on invalid resource ("%s").',
	+ $resource_id));
	+ }
	+
	+ $resource_status = $resource->getStatus();
	+
	+ if ($resource_status == DrydockResourceStatus::STATUS_PENDING) {
	+ // TODO: This is explicitly a temporary failure -- we are waiting for
	+ // the resource to come up.
	+ throw new Exception(pht('Resource still activating.'));
	+ }
	+
	+ if ($resource_status != DrydockResourceStatus::STATUS_OPEN) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht(
	+ 'Trying to activate lease on a dead resource (in status "%s").',
	+ $resource_status));
	+ }
	+
	+ // NOTE: We can race resource destruction here. Between the time we
	+ // performed the read above and now, the resource might have closed, so
	+ // we may activate leases on dead resources. At least for now, this seems
	+ // fine: a resource dying right before we activate a lease on it should not
	+ // be distinguisahble from a resource dying right after we activate a lease
	+ // on it. We end up with an active lease on a dead resource either way, and
	+ // can not prevent resources dying from lightning strikes.
	+
	+ $blueprint = $resource->getBlueprint();
	+ $blueprint->activateLease($resource, $lease);
	+ $this->validateActivatedLease($blueprint, $resource, $lease);
	+ }
	+
	+ private function validateActivatedLease(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource,
	+ DrydockLease $lease) {
	+
	+ if (!$lease->isActivatedLease()) {
	+ throw new Exception(
	+ pht(
	+ 'Blueprint "%s" (of type "%s") is not properly implemented: it '.
	+ 'returned from "%s" without activating a lease.',
	+ $blueprint->getBlueprintName(),
	+ $blueprint->getClassName(),
	+ 'acquireLease()'));
	+ }
	+
	+ }
	+
	+}
	diff --git a/src/applications/drydock/worker/DrydockResourceWorker.php b/src/applications/drydock/worker/DrydockResourceWorker.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/drydock/worker/DrydockResourceWorker.php
	@@ -0,0 +1,45 @@
	+<?php
	+
	+final class DrydockResourceWorker extends DrydockWorker {
	+
	+ protected function doWork() {
	+ $resource_phid = $this->getTaskDataValue('resourcePHID');
	+ $resource = $this->loadResource($resource_phid);
	+
	+ $this->activateResource($resource);
	+ }
	+
	+
	+ private function activateResource(DrydockResource $resource) {
	+ $resource_status = $resource->getStatus();
	+
	+ if ($resource_status != DrydockResourceStatus::STATUS_PENDING) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht(
	+ 'Trying to activate resource from wrong status ("%s").',
	+ $resource_status));
	+ }
	+
	+ $blueprint = $resource->getBlueprint();
	+ $blueprint->activateResource($resource);
	+ $this->validateActivatedResource($blueprint, $resource);
	+ }
	+
	+
	+ private function validateActivatedResource(
	+ DrydockBlueprint $blueprint,
	+ DrydockResource $resource) {
	+
	+ if (!$resource->isActivatedResource()) {
	+ throw new Exception(
	+ pht(
	+ 'Blueprint "%s" (of type "%s") is not properly implemented: %s '.
	+ 'must actually allocate the resource it returns.',
	+ $blueprint->getBlueprintName(),
	+ $blueprint->getClassName(),
	+ 'allocateResource()'));
	+ }
	+
	+ }
	+
	+}
	diff --git a/src/applications/drydock/worker/DrydockWorker.php b/src/applications/drydock/worker/DrydockWorker.php
	new file mode 100644
	--- /dev/null
	+++ b/src/applications/drydock/worker/DrydockWorker.php
	@@ -0,0 +1,39 @@
	+<?php
	+
	+abstract class DrydockWorker extends PhabricatorWorker {
	+
	+ protected function getViewer() {
	+ return PhabricatorUser::getOmnipotentUser();
	+ }
	+
	+ protected function loadLease($lease_phid) {
	+ $viewer = $this->getViewer();
	+
	+ $lease = id(new DrydockLeaseQuery())
	+ ->setViewer($viewer)
	+ ->withPHIDs(array($lease_phid))
	+ ->executeOne();
	+ if (!$lease) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht('No such lease "%s"!', $lease_phid));
	+ }
	+
	+ return $lease;
	+ }
	+
	+ protected function loadResource($resource_phid) {
	+ $viewer = $this->getViewer();
	+
	+ $resource = id(new DrydockResourceQuery())
	+ ->setViewer($viewer)
	+ ->withPHIDs(array($resource_phid))
	+ ->executeOne();
	+ if (!$resource) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht('No such resource "%s"!', $resource_phid));
	+ }
	+
	+ return $resource;
	+ }
	+
	+}
	diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php
	--- a/src/infrastructure/daemon/workers/PhabricatorWorker.php
	+++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php
	@@ -87,6 +87,15 @@
	return $this->data;
	}

	+ final protected function getTaskDataValue($key, $default = null) {
	+ $data = $this->getTaskData();
	+ if (!is_array($data)) {
	+ throw new PhabricatorWorkerPermanentFailureException(
	+ pht('Expected task data to be a dictionary.'));
	+ }
	+ return idx($data, $key, $default);
	+ }
	+
	final public function executeTask() {
	$this->doWork();
	}
	@@ -149,8 +158,7 @@


	/**
	- * Wait for tasks to complete. If tasks are not leased by other workers, they
	- * will be executed in this process while waiting.
	+ * Wait for tasks to complete.
	*
	* @param list<int> List of queued task IDs to wait for.
	* @return void
	@@ -178,24 +186,9 @@
	break;
	}

	- $tasks = id(new PhabricatorWorkerLeaseQuery())
	- ->withIDs($waiting)
	- ->setLimit(1)
	- ->execute();
	-
	- if (!$tasks) {
	- // We were not successful in leasing anything. Sleep for a bit and
	- // see if we have better luck later.
	- sleep(1);
	- continue;
	- }
	-
	- $task = head($tasks)->executeTask();
	-
	- $ex = $task->getExecutionException();
	- if ($ex) {
	- throw $ex;
	- }
	+ // We were not successful in leasing anything. Sleep for a bit and
	+ // see if we have better luck later.
	+ sleep(1);
	}

	$tasks = id(new PhabricatorWorkerArchiveTaskQuery())

File Metadata

Mime Type: text/plain
Expires: Jul 29 2025, 5:28 PM (14 w, 3 d ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/bu/ql/3wnioj24xwadbiz2
Default Alt Text: D14127.diff (34 KB)

D14127.diffNo OneTemporaryActions

D14127.diffView Options

File Metadata

Event Timeline

D14127.diff
No OneTemporary
Actions

D14127.diff
View Options