Don't reclaim resources that have a destroyed lease less than 3 minutes old

Summary:
Ref T13676. The 3-minute grace period when a resource can not be reclaimed after its leases are released currently doesn't work reliably because the Resource object usually isn't actually updated when a lease is released.

Add an additional check for recently-destroyed leases, and extend the grace period if we find any.

Test Plan:
  - See T13676. Ran reproduction sequence there, observed immediate resource reclamation.
  - Applied patch.
  - Ran sequence again, observed repository B wait 3 minutes to reclaim a repository A resource.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

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

src/applications/drydock/query/DrydockLeaseQuery.php

@@ -9,6 +9,8 @@ final class DrydockLeaseQuery extends DrydockQuery {
   private $statuses;
   private $datasourceQuery;
   private $needUnconsumedCommands;
+  private $minModified;
+  private $maxModified;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -40,6 +42,12 @@ final class DrydockLeaseQuery extends DrydockQuery {
     return $this;
   }
 
+  public function withDateModifiedBetween($min_epoch, $max_epoch) {
+    $this->minModified = $min_epoch;
+    $this->maxModified = $max_epoch;
+    return $this;
+  }
+
   public function needUnconsumedCommands($need) {
     $this->needUnconsumedCommands = $need;
     return $this;
@@ -146,6 +154,20 @@ final class DrydockLeaseQuery extends DrydockQuery {
         (int)$this->datasourceQuery);
     }
 
+    if ($this->minModified !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'dateModified >= %d',
+        $this->minModified);
+    }
+
+    if ($this->maxModified !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'dateModified <= %d',
+        $this->maxModified);
+    }
+
     return $where;
   }
 

src/applications/drydock/storage/DrydockLease.php

@@ -111,6 +111,9 @@ final class DrydockLease extends DrydockDAO
         'key_owner' => array(
           'columns' => array('ownerPHID'),
         ),
+        'key_recent' => array(
+          'columns' => array('resourcePHID', 'dateModified'),
+        ),
       ),
     ) + parent::getConfiguration();
   }

src/applications/drydock/worker/DrydockWorker.php

@@ -203,14 +203,15 @@ abstract class DrydockWorker extends PhabricatorWorker {
     // from one another forever without making progress, so make resources
     // immune to reclamation for a little while after they activate or update.
 
+    $now = PhabricatorTime::getNow();
+    $max_epoch = ($now - phutil_units('3 minutes in seconds'));
+
     // TODO: It would be nice to use a more narrow time here, like "last
     // activation or lease release", but we don't currently store that
     // anywhere.
 
     $updated = $resource->getDateModified();
-    $now = PhabricatorTime::getNow();
-    $ago = ($now - $updated);
-    if ($ago < phutil_units('3 minutes in seconds')) {
+    if ($updated > $max_epoch) {
       return false;
     }
 
@@ -233,6 +234,21 @@ abstract class DrydockWorker extends PhabricatorWorker {
       return false;
     }
 
+    // See T13676. Don't reclaim a resource if a lease recently released.
+    $leases = id(new DrydockLeaseQuery())
+      ->setViewer($viewer)
+      ->withResourcePHIDs(array($resource->getPHID()))
+      ->withStatuses(
+        array(
+          DrydockLeaseStatus::STATUS_DESTROYED,
+        ))
+      ->withDateModifiedBetween($max_epoch, null)
+      ->setLimit(1)
+      ->execute();
+    if ($leases) {
+      return false;
+    }
+
     return true;
   }