Modularize the Garbage Collector

Summary:
The GC is a big block of hard-coded application GCs right now. Among other things, this means third parties can't tap into the infrastructure.

Modularize it into `GarbageCollector` classes. This implements only one to prove the new stuff works; I'll followup with the rest in the next diff or few depending on how much mess I run into.

Test Plan: Used `bin/phd debug garbage` to run the collector in debug mode, observed reasonable output and behavior.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

src/__phutil_library_map__.php

@@ -816,6 +816,7 @@ phutil_register_library_map(array(
     'HeraldTransactionQuery' => 'applications/herald/query/HeraldTransactionQuery.php',
     'HeraldTranscript' => 'applications/herald/storage/transcript/HeraldTranscript.php',
     'HeraldTranscriptController' => 'applications/herald/controller/HeraldTranscriptController.php',
+    'HeraldTranscriptGarbageCollector' => 'applications/herald/garbagecollector/HeraldTranscriptGarbageCollector.php',
     'HeraldTranscriptListController' => 'applications/herald/controller/HeraldTranscriptListController.php',
     'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php',
     'HeraldTranscriptTestCase' => 'applications/herald/storage/__tests__/HeraldTranscriptTestCase.php',
@@ -1500,8 +1501,9 @@ phutil_register_library_map(array(
     'PhabricatorFlaggableInterface' => 'applications/flag/interface/PhabricatorFlaggableInterface.php',
     'PhabricatorFlagsUIEventListener' => 'applications/flag/events/PhabricatorFlagsUIEventListener.php',
     'PhabricatorFormExample' => 'applications/uiexample/examples/PhabricatorFormExample.php',
+    'PhabricatorGarbageCollector' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php',
     'PhabricatorGarbageCollectorConfigOptions' => 'applications/config/option/PhabricatorGarbageCollectorConfigOptions.php',
-    'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php',
+    'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php',
     'PhabricatorGestureExample' => 'applications/uiexample/examples/PhabricatorGestureExample.php',
     'PhabricatorGitGraphStream' => 'applications/repository/daemon/PhabricatorGitGraphStream.php',
     'PhabricatorGlobalLock' => 'infrastructure/util/PhabricatorGlobalLock.php',
@@ -3324,6 +3326,7 @@ phutil_register_library_map(array(
       1 => 'PhabricatorPolicyInterface',
     ),
     'HeraldTranscriptController' => 'HeraldController',
+    'HeraldTranscriptGarbageCollector' => 'PhabricatorGarbageCollector',
     'HeraldTranscriptListController' => 'HeraldController',
     'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HeraldTranscriptTestCase' => 'PhabricatorTestCase',
@@ -4112,6 +4115,7 @@ phutil_register_library_map(array(
     'PhabricatorFlaggableInterface' => 'PhabricatorPHIDInterface',
     'PhabricatorFlagsUIEventListener' => 'PhabricatorEventListener',
     'PhabricatorFormExample' => 'PhabricatorUIExample',
+    'PhabricatorGarbageCollector' => 'Phobject',
     'PhabricatorGarbageCollectorConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorGarbageCollectorDaemon' => 'PhabricatorDaemon',
     'PhabricatorGestureExample' => 'PhabricatorUIExample',

src/applications/herald/garbagecollector/HeraldTranscriptGarbageCollector.php

@@ -0,0 +1,31 @@
+<?php
+
+final class HeraldTranscriptGarbageCollector
+  extends PhabricatorGarbageCollector {
+
+  public function collectGarbage() {
+    $ttl = PhabricatorEnv::getEnvConfig('gcdaemon.ttl.herald-transcripts');
+    if ($ttl <= 0) {
+      return false;
+    }
+
+    $table = new HeraldTranscript();
+    $conn_w = $table->establishConnection('w');
+
+    queryfx(
+      $conn_w,
+      'UPDATE %T SET
+          objectTranscript     = "",
+          ruleTranscripts      = "",
+          conditionTranscripts = "",
+          applyTranscripts     = "",
+          garbageCollected     = 1
+        WHERE garbageCollected = 0 AND `time` < %d
+        LIMIT 100',
+      $table->getTableName(),
+      time() - $ttl);
+
+    return ($conn_w->getAffectedRows() == 100);
+  }
+
+}

src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php

@@ -0,0 +1,12 @@
+<?php
+
+abstract class PhabricatorGarbageCollector extends Phobject {
+
+  /**
+   * Collect garbage from whatever source this GC handles.
+   *
+   * @return bool True if there is more garbage to collect.
+   */
+  abstract public function collectGarbage();
+
+}

src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php

@@ -9,8 +9,11 @@
 final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon {
 
   public function run() {
+    $collectors = id(new PhutilSymbolLoader())
+      ->setAncestorClass('PhabricatorGarbageCollector')
+      ->loadObjects();
+
     do {
-      $n_herald = $this->collectHeraldTranscripts();
       $n_daemon = $this->collectDaemonLogs();
       $n_parse  = $this->collectParseCaches();
       $n_markup = $this->collectMarkupCaches();
@@ -22,7 +25,6 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon {
       $n_ccons  = $this->collectExpiredConduitConnections();
 
       $collected = array(
-        'Herald Transcript'           => $n_herald,
         'Daemon Log'                  => $n_daemon,
         'Differential Parse Cache'    => $n_parse,
         'Markup Cache'                => $n_markup,
@@ -41,6 +43,32 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon {
       }
 
       $total = array_sum($collected);
+
+      // TODO: This logic is unnecessarily complex for now to facilitate a
+      // gradual conversion to the new GC infrastructure.
+
+      $had_more_garbage = false;
+      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();
+          if ($more_garbage) {
+            $had_more_garbage = true;
+          }
+          $this->stillWorking();
+        } while ($more_garbage);
+      }
+
+      if ($had_more_garbage) {
+        $total += 100;
+      }
+
       if ($total < 100) {
         // We didn't max out any of the GCs so we're basically caught up. Ease
         // off the GC loop so we don't keep doing table scans just to delete
@@ -53,31 +81,6 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon {
 
   }
 
-  private function collectHeraldTranscripts() {
-    $ttl = PhabricatorEnv::getEnvConfig('gcdaemon.ttl.herald-transcripts');
-    if ($ttl <= 0) {
-      return 0;
-    }
-
-    $table = new HeraldTranscript();
-    $conn_w = $table->establishConnection('w');
-
-    queryfx(
-      $conn_w,
-      'UPDATE %T SET
-          objectTranscript     = "",
-          ruleTranscripts      = "",
-          conditionTranscripts = "",
-          applyTranscripts     = "",
-          garbageCollected     = 1
-        WHERE garbageCollected = 0 AND `time` < %d
-        LIMIT 100',
-      $table->getTableName(),
-      time() - $ttl);
-
-    return $conn_w->getAffectedRows();
-  }
-
   private function collectDaemonLogs() {
     $ttl = PhabricatorEnv::getEnvConfig('gcdaemon.ttl.daemon-logs');
     if ($ttl <= 0) {