From e9309fdd6ada13e3e1dcfaac7dedb8c9e387239a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 24 Oct 2018 07:32:19 -0700
Subject: [PATCH] When a Drydock lease schedules a resource to be reclaimed,
 awaken the lease update task when the reclaim completes

Summary:
Depends on D19751. Ref T13210. When Drydock needs to reclaim an existing unused resource in order to build a new resource to satisfy a lease, the lease which triggered the reclaim currently gets thrown back into the pool with a 15-second yield.

If the queue is pretty empty and the reclaim is quick, this can mean that we spend up to 15 extra seconds just waiting for the lease update task to get another shot at building a resource (the resource reclaim may complete in a second or two, but nothing else happens until the yield ends).

Instead, when a lease triggers a reclaim, have the reclaim reawaken the lease task when it completes. In the best case, this saves us 15 seconds of waiting. In other cases (the task already completed some other way, the resource gets claimed before the lease gets to it), it's harmless.

Test Plan:
  - Allocated A, A, A working copies with limit 3. Leased a B working copy.
  - Before patch: allocation took ~32 seconds.
  - After patch: allocation takes ~17 seconds (i.e., about 15 seconds less).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210

Differential Revision: https://secure.phabricator.com/D19752
---
 .../autopatches/20181024.drydock.01.commandprops.sql |  2 ++
 .../20181024.drydock.02.commanddefaults.sql          |  2 ++
 src/applications/drydock/storage/DrydockCommand.php  | 12 ++++++++++++
 .../drydock/worker/DrydockResourceUpdateWorker.php   |  7 +++++++
 src/applications/drydock/worker/DrydockWorker.php    |  5 +++++
 5 files changed, 28 insertions(+)
 create mode 100644 resources/sql/autopatches/20181024.drydock.01.commandprops.sql
 create mode 100644 resources/sql/autopatches/20181024.drydock.02.commanddefaults.sql

diff --git a/resources/sql/autopatches/20181024.drydock.01.commandprops.sql b/resources/sql/autopatches/20181024.drydock.01.commandprops.sql
new file mode 100644
index 0000000000..e808146b02
--- /dev/null
+++ b/resources/sql/autopatches/20181024.drydock.01.commandprops.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_drydock.drydock_command
+  ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};
diff --git a/resources/sql/autopatches/20181024.drydock.02.commanddefaults.sql b/resources/sql/autopatches/20181024.drydock.02.commanddefaults.sql
new file mode 100644
index 0000000000..2c336dc40e
--- /dev/null
+++ b/resources/sql/autopatches/20181024.drydock.02.commanddefaults.sql
@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_drydock.drydock_command
+  SET properties = '{}' WHERE properties = '';
diff --git a/src/applications/drydock/storage/DrydockCommand.php b/src/applications/drydock/storage/DrydockCommand.php
index e7d003bdd6..0f7f253217 100644
--- a/src/applications/drydock/storage/DrydockCommand.php
+++ b/src/applications/drydock/storage/DrydockCommand.php
@@ -11,6 +11,7 @@ final class DrydockCommand
   protected $targetPHID;
   protected $command;
   protected $isConsumed;
+  protected $properties = array();
 
   private $commandTarget = self::ATTACHABLE;
 
@@ -22,6 +23,9 @@ final class DrydockCommand
 
   protected function getConfiguration() {
     return array(
+      self::CONFIG_SERIALIZATION => array(
+        'properties' => self::SERIALIZATION_JSON,
+      ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'command' => 'text32',
         'isConsumed' => 'bool',
@@ -43,6 +47,14 @@ final class DrydockCommand
     return $this->assertAttached($this->commandTarget);
   }
 
+  public function setProperty($key, $value) {
+    $this->properties[$key] = $value;
+    return $this;
+  }
+
+  public function getProperty($key, $default = null) {
+    return idx($this->properties, $key, $default);
+  }
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
diff --git a/src/applications/drydock/worker/DrydockResourceUpdateWorker.php b/src/applications/drydock/worker/DrydockResourceUpdateWorker.php
index baa80f90d8..817fbaddee 100644
--- a/src/applications/drydock/worker/DrydockResourceUpdateWorker.php
+++ b/src/applications/drydock/worker/DrydockResourceUpdateWorker.php
@@ -150,6 +150,13 @@ final class DrydockResourceUpdateWorker extends DrydockWorker {
         $this->releaseResource($resource, $reclaimer_phid);
         break;
     }
+
+    // If the command specifies that other worker tasks should be awakened
+    // after it executes, awaken them now.
+    $awaken_ids = $command->getProperty('awakenTaskIDs');
+    if (is_array($awaken_ids) && $awaken_ids) {
+      PhabricatorWorker::awakenTaskIDs($awaken_ids);
+    }
   }
 
 
diff --git a/src/applications/drydock/worker/DrydockWorker.php b/src/applications/drydock/worker/DrydockWorker.php
index 443780680d..8a81da09b4 100644
--- a/src/applications/drydock/worker/DrydockWorker.php
+++ b/src/applications/drydock/worker/DrydockWorker.php
@@ -241,10 +241,15 @@ abstract class DrydockWorker extends PhabricatorWorker {
     DrydockLease $lease) {
     $viewer = $this->getViewer();
 
+    // When the resource releases, we we want to reawaken this task since it
+    // should be able to start building a new resource right away.
+    $worker_task_id = $this->getCurrentWorkerTaskID();
+
     $command = DrydockCommand::initializeNewCommand($viewer)
       ->setTargetPHID($resource->getPHID())
       ->setAuthorPHID($lease->getPHID())
       ->setCommand(DrydockCommand::COMMAND_RECLAIM)
+      ->setProperty('awakenTaskIDs', array($worker_task_id))
       ->save();
 
     $resource->scheduleUpdate();