Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15419329
D14118.id34154.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D14118.id34154.diff
View Options
diff --git a/resources/sql/autopatches/20150916.drydock.slotlocks.1.sql b/resources/sql/autopatches/20150916.drydock.slotlocks.1.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20150916.drydock.slotlocks.1.sql
@@ -0,0 +1,8 @@
+CREATE TABLE {$NAMESPACE}_drydock.drydock_slotlock (
+ id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ ownerPHID VARBINARY(64) NOT NULL,
+ lockIndex BINARY(12) NOT NULL,
+ lockKey LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT},
+ UNIQUE KEY `key_lock` (lockIndex),
+ KEY `key_owner` (ownerPHID)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};
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
@@ -863,6 +863,7 @@
'DrydockResourceViewController' => 'applications/drydock/controller/DrydockResourceViewController.php',
'DrydockSFTPFilesystemInterface' => 'applications/drydock/interface/filesystem/DrydockSFTPFilesystemInterface.php',
'DrydockSSHCommandInterface' => 'applications/drydock/interface/command/DrydockSSHCommandInterface.php',
+ 'DrydockSlotLock' => 'applications/drydock/storage/DrydockSlotLock.php',
'DrydockWebrootInterface' => 'applications/drydock/interface/webroot/DrydockWebrootInterface.php',
'DrydockWorkingCopyBlueprintImplementation' => 'applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php',
'FeedConduitAPIMethod' => 'applications/feed/conduit/FeedConduitAPIMethod.php',
@@ -4585,6 +4586,7 @@
'DrydockResourceViewController' => 'DrydockResourceController',
'DrydockSFTPFilesystemInterface' => 'DrydockFilesystemInterface',
'DrydockSSHCommandInterface' => 'DrydockCommandInterface',
+ 'DrydockSlotLock' => 'DrydockDAO',
'DrydockWebrootInterface' => 'DrydockInterface',
'DrydockWorkingCopyBlueprintImplementation' => 'DrydockBlueprintImplementation',
'FeedConduitAPIMethod' => 'ConduitAPIMethod',
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
@@ -67,14 +67,13 @@
$device = $binding->getDevice();
$device_name = $device->getName();
+ $binding_phid = $binding->getPHID();
+
$resource = $this->newResourceTemplate($blueprint, $device_name)
->setActivateWhenAllocated(true)
->setAttribute('almanacServicePHID', $binding->getServicePHID())
- ->setAttribute('almanacBindingPHID', $binding->getPHID());
-
- // TODO: This algorithm can race, and the "free" binding may not be
- // free by the time we acquire it. Do slot-locking here if that works
- // out, or some other kind of locking if it does not.
+ ->setAttribute('almanacBindingPHID', $binding_phid)
+ ->needSlotLock("almanac.host.binding({$binding_phid})");
try {
return $resource->allocateResource(DrydockResourceStatus::STATUS_OPEN);
@@ -93,8 +92,11 @@
DrydockResource $resource,
DrydockLease $lease) {
- // TODO: We'll currently lease each resource an unlimited number of times,
- // but should stop doing that.
+ // 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.
return true;
}
@@ -104,11 +106,11 @@
DrydockResource $resource,
DrydockLease $lease) {
- // TODO: Once we have limit rules, we should perform slot locking (or other
- // kinds of locking) here.
+ $resource_phid = $resource->getPHID();
$lease
->setActivateWhenAcquired(true)
+ ->needSlotLock("almanac.host.lease({$resource_phid})")
->acquireOnResource($resource);
}
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
@@ -106,8 +106,12 @@
DrydockLease $lease);
final public function releaseLease(
+ DrydockBlueprint $blueprint,
DrydockResource $resource,
DrydockLease $lease) {
+
+ // TODO: This is all broken nonsense.
+
$scope = $this->pushActiveScope(null, $lease);
$released = false;
@@ -117,6 +121,7 @@
$lease->reload();
if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACTIVE) {
+ $lease->release();
$lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED);
$lease->save();
$released = true;
@@ -293,6 +298,7 @@
$resource = id(new DrydockResource())
->setBlueprintPHID($blueprint->getPHID())
+ ->attachBlueprint($blueprint)
->setType($this->getType())
->setStatus(DrydockResourceStatus::STATUS_PENDING)
->setName($name);
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
@@ -163,6 +163,17 @@
}
+ /**
+ * @task lease
+ */
+ public function releaseLease(
+ DrydockResource $resource,
+ DrydockLease $lease) {
+ $this->getImplementation()->releaseLease($this, $resource, $lease);
+ return $this;
+ }
+
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
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
@@ -15,6 +15,7 @@
private $releaseOnDestruction;
private $isAcquired = false;
private $activateWhenAcquired = false;
+ private $slotLocks = array();
/**
* Flag this lease to be released when its destructor is called. This is
@@ -128,6 +129,8 @@
$this->setStatus(DrydockLeaseStatus::STATUS_RELEASED);
$this->save();
+ DrydockSlotLock::releaseLocks($this->getPHID());
+
$this->resource = null;
return $this;
@@ -206,6 +209,11 @@
return $this;
}
+ public function needSlotLock($key) {
+ $this->slotLocks[] = $key;
+ return $this;
+ }
+
public function acquireOnResource(DrydockResource $resource) {
$expect_status = DrydockLeaseStatus::STATUS_PENDING;
$actual_status = $this->getStatus();
@@ -234,10 +242,17 @@
}
}
- $this
- ->setResourceID($resource->getID())
- ->setStatus($new_status)
- ->save();
+ $this->openTransaction();
+
+ $this
+ ->setResourceID($resource->getID())
+ ->setStatus($new_status)
+ ->save();
+
+ DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
+ $this->slotLocks = array();
+
+ $this->saveTransaction();
$this->isAcquired = true;
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
@@ -17,6 +17,7 @@
private $blueprint = self::ATTACHABLE;
private $isAllocated = false;
private $activateWhenAllocated = false;
+ private $slotLocks = array();
protected function getConfiguration() {
return array(
@@ -80,6 +81,11 @@
return $this;
}
+ public function needSlotLock($key) {
+ $this->slotLocks[] = $key;
+ return $this;
+ }
+
public function allocateResource($status) {
if ($this->getID()) {
throw new Exception(
@@ -105,11 +111,18 @@
$new_status = DrydockResourceStatus::STATUS_PENDING;
}
- $this
- ->setStatus($new_status)
- ->save();
+ $this->openTransaction();
+
+ $this
+ ->setStatus($new_status)
+ ->save();
- $this->didAllocate = true;
+ DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
+ $this->slotLocks = array();
+
+ $this->saveTransaction();
+
+ $this->isAllocated = true;
return $this;
}
@@ -151,6 +164,9 @@
$this->setStatus(DrydockResourceStatus::STATUS_CLOSED);
$this->save();
+
+ DrydockSlotLock::releaseLocks($this->getPHID());
+
$this->saveTransaction();
}
diff --git a/src/applications/drydock/storage/DrydockSlotLock.php b/src/applications/drydock/storage/DrydockSlotLock.php
new file mode 100644
--- /dev/null
+++ b/src/applications/drydock/storage/DrydockSlotLock.php
@@ -0,0 +1,107 @@
+<?php
+
+/**
+ * Simple optimistic locks for Drydock resources and leases.
+ *
+ * Most blueprints only need very simple locks: for example, a host blueprint
+ * might not want to create multiple resources representing the same physical
+ * machine. These optimistic "slot locks" provide a flexible way to do this
+ * sort of simple locking.
+ *
+ * @task lock Acquiring and Releasing Locks
+ */
+final class DrydockSlotLock extends DrydockDAO {
+
+ protected $ownerPHID;
+ protected $lockIndex;
+ protected $lockKey;
+
+ protected function getConfiguration() {
+ return array(
+ self::CONFIG_TIMESTAMPS => false,
+ self::CONFIG_COLUMN_SCHEMA => array(
+ 'lockIndex' => 'bytes12',
+ 'lockKey' => 'text',
+ ),
+ self::CONFIG_KEY_SCHEMA => array(
+ 'key_lock' => array(
+ 'columns' => array('lockIndex'),
+ 'unique' => true,
+ ),
+ 'key_owner' => array(
+ 'columns' => array('ownerPHID'),
+ ),
+ ),
+ ) + parent::getConfiguration();
+ }
+
+ public static function loadLocks($owner_phid) {
+ return id(new DrydockSlotLock())->loadAllWhere(
+ 'ownerPHID = %s',
+ $owner_phid);
+ }
+
+
+/* -( Acquiring and Releasing Locks )-------------------------------------- */
+
+
+ /**
+ * Acquire a set of slot locks.
+ *
+ * This method either acquires all the locks or throws an exception (usually
+ * because one or more locks are held).
+ *
+ * @param phid Lock owner PHID.
+ * @param list<string> List of locks to acquire.
+ * @return void
+ * @task locks
+ */
+ public static function acquireLocks($owner_phid, array $locks) {
+ if (!$locks) {
+ return;
+ }
+
+ $table = new DrydockSlotLock();
+ $conn_w = $table->establishConnection('w');
+
+ $sql = array();
+ foreach ($locks as $lock) {
+ $sql[] = qsprintf(
+ $conn_w,
+ '(%s, %s, %s)',
+ $owner_phid,
+ PhabricatorHash::digestForIndex($lock),
+ $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));
+ }
+
+
+ /**
+ * Release all locks held by an owner.
+ *
+ * @param phid Lock owner PHID.
+ * @return void
+ * @task locks
+ */
+ public static function releaseLocks($owner_phid) {
+ $table = new DrydockSlotLock();
+ $conn_w = $table->establishConnection('w');
+
+ queryfx(
+ $conn_w,
+ 'DELETE FROM %T WHERE ownerPHID = %s',
+ $table->getTableName(),
+ $owner_phid);
+ }
+
+}
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
@@ -350,7 +350,7 @@
DrydockBlueprint $blueprint,
DrydockLease $lease) {
$resource = $blueprint->allocateResource($lease);
- $this->validateAllocatedResource($resource);
+ $this->validateAllocatedResource($blueprint, $resource, $lease);
return $resource;
}
@@ -369,7 +369,6 @@
DrydockBlueprint $blueprint,
$resource,
DrydockLease $lease) {
- $blueprint = $this->getBlueprintClass();
if (!($resource instanceof DrydockResource)) {
throw new Exception(
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Mar 22, 5:21 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7710041
Default Alt Text
D14118.id34154.diff (12 KB)
Attached To
Mode
D14118: Implement optimistic "slot locks" in Drydock
Attached
Detach File
Event Timeline
Log In to Comment