Allow Drydock Blueprints to control "supplemental allocation" behavior so all hosts in an Almanac pool get used

Summary:
Fixes T12145. Ref T13210. See PHI570. See PHI536.

Currently, when you give Drydock an Almanac host pool with more than one host, it never voluntarily builds a second host resource: there is no way to say "maximum X working copies per host" (only "maximum X global working copies") to make the first host overflow, and the allocator tries to pack resources as tightly as possible.

If you can force it to allocate the 2nd..Nth host, things will work reasonably well from there (it will spread working copies across the hosts randomly), but tricking it is very hard, especially before D19761.

To deal with this, give blueprints a new behavior around "supplemental allocations". The idea here is that a blueprint may decide that it would prefer to allocate a fresh new resource instead of allowing an otherwise valid acquisition to occur.

These supplemental allocations follow all the normal allocation rules (they can't exceed limits or actually replace existing resources), so they can only happen if there's free space in the resource pool. But a blueprint can elect for a supplemental allocation to provide a "grow the pool" hint.

The only useful policies here are probably "true" (immediately use all resources, like Almanac) or "false" (pack resources as efficiently as possible) but some other policies //might// be useful (perhaps "start growing the pool when we're getting a bit full even if we aren't at the limit yet, since our workload is bursty").

Then, give Almanac host resources a "true" policy (always allocate supplemental resources) so they use all hosts once a similar number of concurrent jobs arrive.

One aspect of this approach is that we only do supplemental resources if the normal allocation algorithm already decided that the best resource to acquire was part of the same blueprint. I started with an approach like "look at all the blueprints and see if any of them want to be greedy", but then a not-very-desirable blueprint would end up filling up its whole pool before we skipped the supplemental allocation part and ended up picking a different resource. That felt a bit silly and this feels a little cleaner and more focused.

Test Plan:
  - Without changing the Almanac blueprint policy, allocated hosts. Got A, A, A, A, ... (second host never used).
  - Changed the Almanac policy.
  - Allocated hosts, got A, B, random mix of A and B.
  - Destroyed B. Destroyed all leases on A. Allocated. Got A. This tests the "don't build a supplemental resource if there are no leases on the natural resource".

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210, T12145

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

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php

@@ -45,6 +45,16 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     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) {

src/applications/drydock/blueprint/DrydockBlueprintImplementation.php

@@ -139,6 +139,38 @@ abstract class DrydockBlueprintImplementation extends Phobject {
     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  )------------------------------------------------ */
 

src/applications/drydock/storage/DrydockBlueprint.php

@@ -278,6 +278,15 @@ final class DrydockBlueprint extends DrydockDAO
     return $interface;
   }
 
+  public function shouldAllocateSupplementalResource(
+    DrydockResource $resource,
+    DrydockLease $lease) {
+    return $this->getImplementation()->shouldAllocateSupplementalResource(
+      $this,
+      $resource,
+      $lease);
+  }
+
 
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
 

src/applications/drydock/worker/DrydockLeaseUpdateWorker.php

@@ -306,6 +306,7 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     $allocated = false;
     foreach ($resources as $resource) {
       try {
+        $resource = $this->newResourceForAcquisition($resource, $lease);
         $this->acquireLease($resource, $lease);
         $allocated = true;
         break;
@@ -319,6 +320,10 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
         // 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 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     }
   }
 
+  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  )-------------------------------------------------- */