Merge GC daemon into Trigger daemon

Summary:
Fixes T7352. This reduces the memory footprint for instances by combining these two similar daemons into one daemon which handles the responsibilities of both.

The fit isn't 100% perfect here but it's pretty close, and the GC daemon is fairly trivial.

Test Plan:
  - Adjusted all the numbers to small numbers (5 second sleep, 120 second GC length).
  - Added a ton of logging.
  - Started trigger daemon.
    - Saw it run a GC cycle.
    - Saw it reschedule another cycle after 120 seconds (adjusted down from 4 hours).
  - Reverted all the logging/small numbers.
  - Ran `bin/phd start`, saw stable trigger daemon running.
  - Grepped for removed daemon class name.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7352

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

src/__phutil_library_map__.php

@@ -1838,7 +1838,6 @@ phutil_register_library_map(array(
     'PhabricatorGDSetupCheck' => 'applications/config/check/PhabricatorGDSetupCheck.php',
     'PhabricatorGarbageCollector' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php',
     'PhabricatorGarbageCollectorConfigOptions' => 'applications/config/option/PhabricatorGarbageCollectorConfigOptions.php',
-    'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php',
     'PhabricatorGestureUIExample' => 'applications/uiexample/examples/PhabricatorGestureUIExample.php',
     'PhabricatorGitGraphStream' => 'applications/repository/daemon/PhabricatorGitGraphStream.php',
     'PhabricatorGitHubAuthProvider' => 'applications/auth/provider/PhabricatorGitHubAuthProvider.php',
@@ -5122,7 +5121,6 @@ phutil_register_library_map(array(
     'PhabricatorGDSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorGarbageCollector' => 'Phobject',
     'PhabricatorGarbageCollectorConfigOptions' => 'PhabricatorApplicationConfigOptions',
-    'PhabricatorGarbageCollectorDaemon' => 'PhabricatorDaemon',
     'PhabricatorGestureUIExample' => 'PhabricatorUIExample',
     'PhabricatorGitGraphStream' => 'PhabricatorRepositoryGraphStream',
     'PhabricatorGitHubAuthProvider' => 'PhabricatorOAuth2AuthProvider',

src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php

@@ -337,9 +337,6 @@ abstract class PhabricatorDaemonManagementWorkflow
       array(
         'class' => 'PhabricatorRepositoryPullLocalDaemon',
       ),
-      array(
-        'class' => 'PhabricatorGarbageCollectorDaemon',
-      ),
       array(
         'class' => 'PhabricatorTriggerDaemon',
       ),

src/docs/user/configuration/managing_daemons.diviner

@@ -70,7 +70,8 @@ You can get a list of launchable daemons with **phd list**:
   - **PhabricatorTaskmasterDaemon** performs work from a task queue;
   - **PhabricatorRepositoryPullLocalDaemon** daemons track repositories, for
     more information see @{article:Diffusion User Guide}; and
-  - **PhabricatorGarbageCollectorDaemon** cleans up old logs and caches.
+  - **PhabricatorTriggerDaemon** schedules event triggers and cleans up old
+    logs and caches.
 
 = Debugging and Tuning =
 
@@ -120,7 +121,7 @@ daemons launch, and split daemons across machines like this:
   - `PhabricatorRepositoryPullLocalDaemon`: Run one copy on any machine.
     On each web frontend which is not running a normal copy, run a copy
     with the `--no-discovery` flag.
-  - `PhabricatorGarbageCollectorDaemon`: Run one copy on any machine.
+  - `PhabricatorTriggerDaemon`: Run one copy on any machine.
   - `PhabricatorTaskmasterDaemon`: Run as many copies as you need to keep
     tasks from backing up. You can run them all on one machine or split them
     across machines.

src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php

@@ -1,38 +0,0 @@
-<?php
-
-/**
- * Collects old logs and caches to reduce the amount of data stored in the
- * database.
- */
-final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon {
-
-  protected function run() {
-    $collectors = id(new PhutilSymbolLoader())
-      ->setAncestorClass('PhabricatorGarbageCollector')
-      ->loadObjects();
-
-    do {
-      foreach ($collectors as $name => $collector) {
-        $more_garbage = false;
-        do {
-          if ($more_garbage) {
-            $this->log(pht('Collecting more garbage with "%s".', $name));
-          } else {
-            $this->log(pht('Collecting garbage with "%s".', $name));
-          }
-
-          $more_garbage = $collector->collectGarbage();
-          $this->stillWorking();
-        } while ($more_garbage);
-      }
-
-      // We made it to the end of the run cycle of every GC, so we're more or
-      // less caught up. Ease off the GC loop so we don't keep doing table
-      // scans just to delete a handful of rows; wake up in a few hours.
-      $this->log(pht('All caught up, waiting for more garbage.'));
-      $this->sleep(4 * (60 * 60));
-    } while (!$this->shouldExit());
-
-  }
-
-}

src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php

@@ -2,6 +2,10 @@
 
 /**
  * Schedule and execute event triggers, which run code at specific times.
+ *
+ * Also performs garbage collection of old logs, caches, etc.
+ *
+ * @task garbage Garbage Collection
  */
 final class PhabricatorTriggerDaemon
   extends PhabricatorDaemon {
@@ -9,6 +13,9 @@ final class PhabricatorTriggerDaemon
   const COUNTER_VERSION = 'trigger.version';
   const COUNTER_CURSOR = 'trigger.cursor';
 
+  private $garbageCollectors;
+  private $nextCollection;
+
   protected function run() {
 
     // The trigger daemon is a low-level infrastructure daemon which schedules
@@ -54,6 +61,9 @@ final class PhabricatorTriggerDaemon
     // trying to reschedule events after an update could race with other web
     // processes or the daemon.
 
+    // We want to start the first GC cycle right away, not wait 4 hours.
+    $this->nextCollection = PhabricatorTime::getNow();
+
     do {
       $lock = PhabricatorGlobalLock::newLock('trigger');
 
@@ -86,7 +96,9 @@ final class PhabricatorTriggerDaemon
 
       $lock->unlock();
 
-      $this->sleep($this->getSleepDuration());
+      $sleep_duration = $this->getSleepDuration();
+      $sleep_duration = $this->runGarbageCollection($sleep_duration);
+      $this->sleep($sleep_duration);
     } while (!$this->shouldExit());
   }
 
@@ -248,7 +260,7 @@ final class PhabricatorTriggerDaemon
    * @return int Number of seconds to sleep for.
    */
   private function getSleepDuration() {
-    $sleep = 60;
+    $sleep = 5;
 
     $next_triggers = id(new PhabricatorWorkerTriggerQuery())
       ->setViewer($this->getViewer())
@@ -290,4 +302,91 @@ final class PhabricatorTriggerDaemon
       id(new PhabricatorWorkerTrigger())->establishConnection('w'),
       $counter_name);
   }
+
+
+/* -(  Garbage Collection  )------------------------------------------------- */
+
+
+  /**
+   * Run the garbage collector for up to a specified number of seconds.
+   *
+   * @param int Number of seconds the GC may run for.
+   * @return int Number of seconds remaining in the time budget.
+   * @task garbage
+   */
+  private function runGarbageCollection($duration) {
+    $run_until = (PhabricatorTime::getNow() + $duration);
+
+    // NOTE: We always run at least one GC cycle to make sure the GC can make
+    // progress even if the trigger queue is busy.
+    do {
+      $more_garbage = $this->updateGarbageCollection();
+      if (!$more_garbage) {
+        // If we don't have any more collection work to perform, we're all
+        // done.
+        break;
+      }
+    } while (PhabricatorTime::getNow() <= $run_until);
+
+    $remaining = max(0, $run_until - PhabricatorTime::getNow());
+
+    return $remaining;
+  }
+
+
+  /**
+   * Update garbage collection, possibly collecting a small amount of garbage.
+   *
+   * @return bool True if there is more garbage to collect.
+   * @task garbage
+   */
+  private function updateGarbageCollection() {
+    // If we're ready to start the next collection cycle, load all the
+    // collectors.
+    $next = $this->nextCollection;
+    if ($next && (PhabricatorTime::getNow() >= $next)) {
+      $this->nextCollection = null;
+      $this->garbageCollectors = $this->loadGarbageCollectors();
+    }
+
+    // If we're in a collection cycle, continue collection.
+    if ($this->garbageCollectors) {
+      foreach ($this->garbageCollectors as $key => $collector) {
+        $more_garbage = $collector->collectGarbage();
+        if (!$more_garbage) {
+          unset($this->garbageCollectors[$key]);
+        }
+        // We only run one collection per call, to prevent triggers from being
+        // thrown too far off schedule if there's a lot of garbage to collect.
+        break;
+      }
+
+      if ($this->garbageCollectors) {
+        // If we have more work to do, return true.
+        return true;
+      }
+
+      // Otherwise, reschedule another cycle in 4 hours.
+      $now = PhabricatorTime::getNow();
+      $wait = phutil_units('4 hours in seconds');
+      $this->nextCollection = $now + $wait;
+    }
+
+    return false;
+  }
+
+
+  /**
+   * Load all of the available garbage collectors.
+   *
+   * @return list<PhabricatorGarbageCollector> Garbage collectors.
+   * @task garbage
+   */
+  private function loadGarbageCollectors() {
+    return id(new PhutilSymbolLoader())
+      ->setAncestorClass('PhabricatorGarbageCollector')
+      ->loadObjects();
+  }
+
+
 }