Remove Drydock host resource limits and give working copies simple limits

Summary:
Ref T9252. Right now, we have very strict limits on Drydock: one lease per host, and one working copy per working copy blueprint.

These are silly and getting in the way of using "Land Revision" more widely, since we need at least one working copy for each landable repository.

For now, just remove the host limit and put a simple limit on working copies. This might need to be fancier some day (e.g., limit working copies per-host) but it is generally reasonable for the use cases of today.

Also add a `--background` flag to make testing a little easier.

(Limits are also less important nowadays than they were in the past, because pools expand slowly now and we seem to have stamped out all the "runaway train" bugs where allocators go crazy and allocate a million things.)

Test Plan:
  - With a limit of 5, ran 10 concurrent builds and saw them finish after allocating 5 total resources.
  - Removed limit, raised taskmaster concurrency to 128, ran thousands of builds in blocks of 128 or 256.
    - Saw Drydock gradually expand the pool, allocating a few more working copies at first and a lot of working copies later.
    - Got ~256 builds in ~140 seconds, which isn't a breakneck pace or anything but isn't too bad.
    - This stuff seems to be mostly bottlenecked on `sbuild` throttling inbound SSH connections. I haven't tweaked it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14334

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php

@@ -109,11 +109,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     DrydockBlueprint $blueprint,
     DrydockResource $resource,
     DrydockLease $lease) {
-
-    if (!DrydockSlotLock::isLockFree($this->getLeaseSlotLock($resource))) {
-      return false;
-    }
-
     return true;
   }
 
@@ -124,7 +119,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
 
     $lease
       ->setActivateWhenAcquired(true)
-      ->needSlotLock($this->getLeaseSlotLock($resource))
       ->acquireOnResource($resource);
   }
 
@@ -146,11 +140,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     return;
   }
 
-  private function getLeaseSlotLock(DrydockResource $resource) {
-    $resource_phid = $resource->getPHID();
-    return "almanac.host.lease({$resource_phid})";
-  }
-
   public function getType() {
     return 'host';
   }
@@ -188,7 +177,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     }
   }
 
-  public function getFieldSpecifications() {
+  protected function getCustomFieldSpecifications() {
     return array(
       'almanacServicePHIDs' => array(
         'name' => pht('Almanac Services'),
@@ -207,7 +196,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
         'credential.type' =>
           PassphraseSSHPrivateKeyTextCredentialType::CREDENTIAL_TYPE,
       ),
-    ) + parent::getFieldSpecifications();
+    );
   }
 
   private function loadServices(DrydockBlueprint $blueprint) {

src/applications/drydock/blueprint/DrydockBlueprintImplementation.php

@@ -16,6 +16,26 @@ abstract class DrydockBlueprintImplementation extends Phobject {
   abstract public function getDescription();
 
   public function getFieldSpecifications() {
+    $fields = array();
+
+    $fields += $this->getCustomFieldSpecifications();
+
+    if ($this->shouldUseConcurrentResourceLimit()) {
+      $fields += array(
+        'allocator.limit' => array(
+          'name' => pht('Limit'),
+          'caption' => pht(
+            'Maximum number of resources this blueprint can have active '.
+            'concurrently.'),
+          'type' => 'int',
+        ),
+      );
+    }
+
+    return $fields;
+  }
+
+  protected function getCustomFieldSpecifications() {
     return array();
   }
 
@@ -316,6 +336,85 @@ abstract class DrydockBlueprintImplementation extends Phobject {
   }
 
 
+  /**
+   * Does this implementation use concurrent resource limits?
+   *
+   * Implementations can override this method to opt into standard limit
+   * behavior, which provides a simple concurrent resource limit.
+   *
+   * @return bool True to use limits.
+   */
+  protected function shouldUseConcurrentResourceLimit() {
+    return false;
+  }
+
+
+  /**
+   * Get the effective concurrent resource limit for this blueprint.
+   *
+   * @param DrydockBlueprint Blueprint to get the limit for.
+   * @return int|null Limit, or `null` for no limit.
+   */
+  protected function getConcurrentResourceLimit(DrydockBlueprint $blueprint) {
+    if ($this->shouldUseConcurrentResourceLimit()) {
+      $limit = $blueprint->getFieldValue('allocator.limit');
+      $limit = (int)$limit;
+      if ($limit > 0) {
+        return $limit;
+      } else {
+        return null;
+      }
+    }
+
+    return null;
+  }
+
+
+  protected function getConcurrentResourceLimitSlotLock(
+    DrydockBlueprint $blueprint) {
+
+    $limit = $this->getConcurrentResourceLimit($blueprint);
+    if ($limit === null) {
+      return;
+    }
+
+    $blueprint_phid = $blueprint->getPHID();
+
+    // TODO: This logic shouldn't do anything awful, but is a little silly. It
+    // would be nice to unify the "huge limit" and "small limit" cases
+    // eventually but it's a little tricky.
+
+    // If the limit is huge, just pick a random slot. This is just stopping
+    // us from exploding if someone types a billion zillion into the box.
+    if ($limit > 1024) {
+      $slot = mt_rand(0, $limit - 1);
+      return "allocator({$blueprint_phid}).limit({$slot})";
+    }
+
+    // For reasonable limits, actually check for an available slot.
+    $locks = DrydockSlotLock::loadLocks($blueprint_phid);
+    $locks = mpull($locks, null, 'getLockKey');
+
+    $slots = range(0, $limit - 1);
+    shuffle($slots);
+
+    foreach ($slots as $slot) {
+      $slot_lock = "allocator({$blueprint_phid}).limit({$slot})";
+      if (empty($locks[$slot_lock])) {
+        return $slot_lock;
+      }
+    }
+
+    // If we found no free slot, just return whatever we checked last (which
+    // is just a random slot). There's a small chance we'll get lucky and the
+    // lock will be free by the time we try to take it, but usually we'll just
+    // fail to grab the lock, throw an appropriate lock exception, and get back
+    // on the right path to retry later.
+    return $slot_lock;
+  }
+
+
+
   /**
    * Apply standard limits on resource allocation rate.
    *
@@ -329,7 +428,7 @@ abstract class DrydockBlueprintImplementation extends Phobject {
     // configurable by the blueprint implementation.
 
     // Limit on total number of active resources.
-    $total_limit = 1;
+    $total_limit = $this->getConcurrentResourceLimit($blueprint);
 
     // Always allow at least this many allocations to be in flight at once.
     $min_allowed = 1;
@@ -358,9 +457,11 @@ abstract class DrydockBlueprintImplementation extends Phobject {
 
     // If we're at the limit on total active resources, limit additional
     // allocations.
-    $n_total = ($n_alloc + $n_active + $n_broken + $n_released);
+    if ($total_limit !== null) {
-    if ($n_total >= $total_limit) {
+      $n_total = ($n_alloc + $n_active + $n_broken + $n_released);
-      return true;
+      if ($n_total >= $total_limit) {
+        return true;
+      }
     }
 
     // If the number of in-flight allocations is fewer than the minimum number

src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php

@@ -125,15 +125,7 @@ final class DrydockWorkingCopyBlueprintImplementation
       ->setOwnerPHID($resource_phid)
       ->setAttribute('workingcopy.resourcePHID', $resource_phid)
       ->setAllowedBlueprintPHIDs($blueprint_phids);
-
+    $resource->setAttribute('host.leasePHID', $host_lease->getPHID());
-    $resource
-      ->setAttribute('host.leasePHID', $host_lease->getPHID())
-      ->save();
-
-    $host_lease->queueForActivation();
-
-    // TODO: Add some limits to the number of working copies we can have at
-    // once?
 
     $map = $lease->getAttribute('repositories.map');
     foreach ($map as $key => $value) {
@@ -143,10 +135,18 @@ final class DrydockWorkingCopyBlueprintImplementation
           'phid',
         ));
     }
+    $resource->setAttribute('repositories.map', $map);
 
-    return $resource
+    $slot_lock = $this->getConcurrentResourceLimitSlotLock($blueprint);
-      ->setAttribute('repositories.map', $map)
+    if ($slot_lock !== null) {
-      ->allocateResource();
+      $resource->needSlotLock($slot_lock);
+    }
+
+    $resource->allocateResource();
+
+    $host_lease->queueForActivation();
+
+    return $resource;
   }
 
   public function activateResource(
@@ -393,14 +393,18 @@ final class DrydockWorkingCopyBlueprintImplementation
     return $lease;
   }
 
-  public function getFieldSpecifications() {
+  protected function getCustomFieldSpecifications() {
     return array(
       'blueprintPHIDs' => array(
         'name' => pht('Use Blueprints'),
         'type' => 'blueprints',
         'required' => true,
       ),
-    ) + parent::getFieldSpecifications();
+    );
+  }
+
+  protected function shouldUseConcurrentResourceLimit() {
+    return true;
   }
 
 

src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php

@@ -15,6 +15,12 @@ final class HarbormasterManagementBuildWorkflow
             'param'       => 'id',
             'help'        => pht('ID of build plan to run.'),
           ),
+          array(
+            'name' => 'background',
+            'help' => pht(
+              'Submit builds into the build queue normally instead of '.
+              'running them in the foreground.'),
+          ),
           array(
             'name'        => 'buildable',
             'wildcard'    => true,
@@ -88,7 +94,10 @@ final class HarbormasterManagementBuildWorkflow
       "\n    %s\n\n",
       PhabricatorEnv::getProductionURI('/B'.$buildable->getID()));
 
-    PhabricatorWorker::setRunAllTasksInProcess(true);
+    if (!$args->getArg('background')) {
+      PhabricatorWorker::setRunAllTasksInProcess(true);
+    }
+
     $buildable->applyPlan($plan, array());
 
     $console->writeOut("%s\n", pht('Done.'));