From a3518e19a56539fe739e6779a9592574bd69ce54 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 23 Feb 2015 18:39:01 -0800 Subject: [PATCH] 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 | 2 - .../PhabricatorDaemonManagementWorkflow.php | 3 - .../configuration/managing_daemons.diviner | 5 +- .../PhabricatorGarbageCollectorDaemon.php | 38 ------- .../workers/PhabricatorTriggerDaemon.php | 103 +++++++++++++++++- 5 files changed, 104 insertions(+), 47 deletions(-) delete mode 100644 src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f698d3f6d1..1bb318eea9 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php index 6d523c76fd..c21275b1df 100644 --- a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php +++ b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php @@ -337,9 +337,6 @@ abstract class PhabricatorDaemonManagementWorkflow array( 'class' => 'PhabricatorRepositoryPullLocalDaemon', ), - array( - 'class' => 'PhabricatorGarbageCollectorDaemon', - ), array( 'class' => 'PhabricatorTriggerDaemon', ), diff --git a/src/docs/user/configuration/managing_daemons.diviner b/src/docs/user/configuration/managing_daemons.diviner index e98af36a24..375c1e80d2 100644 --- a/src/docs/user/configuration/managing_daemons.diviner +++ b/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. diff --git a/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php b/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php deleted file mode 100644 index 105baaaf54..0000000000 --- a/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollectorDaemon.php +++ /dev/null @@ -1,38 +0,0 @@ -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()); - - } - -} diff --git a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php index 51352cc21d..2c016c3e2c 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php +++ b/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 Garbage collectors. + * @task garbage + */ + private function loadGarbageCollectors() { + return id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorGarbageCollector') + ->loadObjects(); + } + + }