From 1c62a35710e5048a01f07e6bfc04ec90b2b4057b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 May 2012 15:01:10 -0700 Subject: [PATCH] Run one daemon to pull all working copies, not one daemon per working copy Summary: Allow the pull daemon to take a list of repositories. By default, pull all repositories. Make some effort to respect pull frequencies, although we'll necessarily suffer a bit if running with only one process. NOTE: We still launch one discovery daemon per working copy, so this only cuts the daemon count in half. Test Plan: - Ran `phd debug pulllocal`, verified behavior. - Ran `pull.php P MTEST SVNTEST --trace`, verified it pulled the repos and ran the right commands. - Ran `phd repository-launch-master`, verified the right daemons launched, checked daemon console. - Ran `phd repository-launch-readonly`, verified the right daemon launched, checked daemon console. Reviewers: btrahan, csilvers, davidreuss Reviewed By: csilvers CC: aran Differential Revision: https://secure.phabricator.com/D2418 --- .../daemon/phabricator_daemon_launcher.php | 55 +--- scripts/repository/pull.php | 51 +++ src/__phutil_library_map__.php | 6 +- .../PhabricatorRepositoryGitFetchDaemon.php | 97 ------ .../repository/daemon/gitfetch/__init__.php | 15 - ...abricatorRepositoryMercurialPullDaemon.php | 73 ----- .../daemon/mercurialpull/__init__.php | 13 - .../PhabricatorRepositoryPullLocalDaemon.php | 307 ++++++++++++++++-- .../repository/daemon/pulllocal/__init__.php | 6 +- .../repository/PhabricatorRepository.php | 16 + 10 files changed, 358 insertions(+), 281 deletions(-) create mode 100755 scripts/repository/pull.php delete mode 100644 src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php delete mode 100644 src/applications/repository/daemon/gitfetch/__init__.php delete mode 100644 src/applications/repository/daemon/mercurialpull/PhabricatorRepositoryMercurialPullDaemon.php delete mode 100644 src/applications/repository/daemon/mercurialpull/__init__.php diff --git a/scripts/daemon/phabricator_daemon_launcher.php b/scripts/daemon/phabricator_daemon_launcher.php index c1e1e36970..7a5776fa5d 100755 --- a/scripts/daemon/phabricator_daemon_launcher.php +++ b/scripts/daemon/phabricator_daemon_launcher.php @@ -2,7 +2,7 @@ getName(); - $callsign = $repository->getCallsign(); - $desc = "'{$name}' ({$callsign})"; - $phid = $repository->getPHID(); - - echo "Launching 'git fetch' daemon on the {$desc} repository...\n"; - $control->launchDaemon( - 'PhabricatorRepositoryGitFetchDaemon', - array( - $phid, - )); - } + exit(0); } + + will_launch($control); + $control->launchDaemon( + 'PhabricatorRepositoryPullLocalDaemon', + array()); break; case 'repository-launch-master': $need_launch = phd_load_tracked_repositories(); if (!$need_launch) { echo "There are no repositories with tracking enabled.\n"; + exit(1); } else { will_launch($control); + $control->launchDaemon( + 'PhabricatorRepositoryPullLocalDaemon', + array()); + foreach ($need_launch as $repository) { $name = $repository->getName(); $callsign = $repository->getCallsign(); @@ -86,12 +81,6 @@ switch (isset($argv[1]) ? $argv[1] : 'help') { switch ($repository->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - echo "Launching 'git fetch' daemon on the {$desc} repository...\n"; - $control->launchDaemon( - 'PhabricatorRepositoryGitFetchDaemon', - array( - $phid, - )); echo "Launching discovery daemon on the {$desc} repository...\n"; $control->launchDaemon( 'PhabricatorRepositoryGitCommitDiscoveryDaemon', @@ -108,12 +97,6 @@ switch (isset($argv[1]) ? $argv[1] : 'help') { )); break; case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - echo "Launching 'hg pull' daemon on the {$desc} repository...\n"; - $control->launchDaemon( - 'PhabricatorRepositoryMercurialPullDaemon', - array( - $phid, - )); echo "Launching discovery daemon on the {$desc} repository...\n"; $control->launchDaemon( 'PhabricatorRepositoryMercurialCommitDiscoveryDaemon', @@ -224,18 +207,6 @@ switch (isset($argv[1]) ? $argv[1] : 'help') { exit($err); } -function phd_load_tracked_repositories_of_type($type) { - $repositories = phd_load_tracked_repositories(); - - foreach ($repositories as $key => $repository) { - if ($repository->getVersionControlSystem() != $type) { - unset($repositories[$key]); - } - } - - return $repositories; -} - function phd_load_tracked_repositories() { phutil_require_module( 'phabricator', diff --git a/scripts/repository/pull.php b/scripts/repository/pull.php new file mode 100755 index 0000000000..668c005f7e --- /dev/null +++ b/scripts/repository/pull.php @@ -0,0 +1,51 @@ +#!/usr/bin/env php +setTagline('manually pull working copies'); +$args->setSynopsis(<<parseStandardArguments(); +$args->parse( + array( + array( + 'name' => 'repositories', + 'wildcard' => true, + ), + )); + +$repo_names = $args->getArg('repositories'); +if (!$repo_names) { + echo "Specify one or more repositories to pull, by callsign or PHID.\n"; + exit(1); +} + +$repos = PhabricatorRepository::loadAllByPHIDOrCallsign($repo_names); +foreach ($repos as $repo) { + $callsign = $repo->getCallsign(); + echo "Pulling '{$callsign}'...\n"; + PhabricatorRepositoryPullLocalDaemon::pullRepository($repo); +} +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d02070ce58..d27cffbc88 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -857,12 +857,10 @@ phutil_register_library_map(array( 'PhabricatorRepositoryGitCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/git', 'PhabricatorRepositoryGitCommitDiscoveryDaemonTestCase' => 'applications/repository/daemon/commitdiscovery/git/__tests__', 'PhabricatorRepositoryGitCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/git', - 'PhabricatorRepositoryGitFetchDaemon' => 'applications/repository/daemon/gitfetch', 'PhabricatorRepositoryListController' => 'applications/repository/controller/list', 'PhabricatorRepositoryMercurialCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/mercurial', 'PhabricatorRepositoryMercurialCommitDiscoveryDaemon' => 'applications/repository/daemon/commitdiscovery/mercurial', 'PhabricatorRepositoryMercurialCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/mercurial', - 'PhabricatorRepositoryMercurialPullDaemon' => 'applications/repository/daemon/mercurialpull', 'PhabricatorRepositoryPullLocalDaemon' => 'applications/repository/daemon/pulllocal', 'PhabricatorRepositoryShortcut' => 'applications/repository/storage/shortcut', 'PhabricatorRepositorySvnCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/svn', @@ -1752,13 +1750,11 @@ phutil_register_library_map(array( 'PhabricatorRepositoryGitCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon', 'PhabricatorRepositoryGitCommitDiscoveryDaemonTestCase' => 'PhabricatorTestCase', 'PhabricatorRepositoryGitCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker', - 'PhabricatorRepositoryGitFetchDaemon' => 'PhabricatorRepositoryPullLocalDaemon', 'PhabricatorRepositoryListController' => 'PhabricatorRepositoryController', 'PhabricatorRepositoryMercurialCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker', 'PhabricatorRepositoryMercurialCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon', 'PhabricatorRepositoryMercurialCommitMessageParserWorker' => 'PhabricatorRepositoryCommitMessageParserWorker', - 'PhabricatorRepositoryMercurialPullDaemon' => 'PhabricatorRepositoryPullLocalDaemon', - 'PhabricatorRepositoryPullLocalDaemon' => 'PhabricatorRepositoryDaemon', + 'PhabricatorRepositoryPullLocalDaemon' => 'PhabricatorDaemon', 'PhabricatorRepositoryShortcut' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositorySvnCommitChangeParserWorker' => 'PhabricatorRepositoryCommitChangeParserWorker', 'PhabricatorRepositorySvnCommitDiscoveryDaemon' => 'PhabricatorRepositoryCommitDiscoveryDaemon', diff --git a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php b/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php deleted file mode 100644 index cc42b714dc..0000000000 --- a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php +++ /dev/null @@ -1,97 +0,0 @@ -execxRemoteCommand( - 'clone --origin origin %s %s', - $repository->getRemoteURI(), - rtrim($local_path, '/')); - } - - protected function executeUpdate( - PhabricatorRepository $repository, - $local_path) { - - // Run a bunch of sanity checks to detect people checking out repositories - // inside other repositories, making empty directories, pointing the local - // path at some random file or path, etc. - - list($err, $stdout) = $repository->execLocalCommand( - 'rev-parse --show-toplevel'); - - if ($err) { - - // Try to raise a more tailored error message in the more common case - // of the user creating an empty directory. (We could try to remove it, - // but might not be able to, and it's much simpler to raise a good - // message than try to navigate those waters.) - if (is_dir($local_path)) { - $files = Filesystem::listDirectory($local_path, $include_hidden = true); - if (!$files) { - throw new Exception( - "Expected to find a git repository at '{$local_path}', but there ". - "is an empty directory there. Remove the directory: the daemon ". - "will run 'git clone' for you."); - } - } - - throw new Exception( - "Expected to find a git repository at '{$local_path}', but there is ". - "a non-repository directory (with other stuff in it) there. Move or ". - "remove this directory (or reconfigure the repository to use a ". - "different directory), and then either clone a repository yourself ". - "or let the daemon do it."); - } else { - $repo_path = rtrim($stdout, "\n"); - - if (empty($repo_path)) { - throw new Exception( - "Expected to find a git repository at '{$local_path}', but ". - "there was no result from `git rev-parse --show-toplevel`. ". - "Something is misconfigured or broken. The git repository ". - "may be inside a '.git/' directory."); - } - - if (!Filesystem::pathsAreEquivalent($repo_path, $local_path)) { - throw new Exception( - "Expected to find repo at '{$local_path}', but the actual ". - "git repository root for this directory is '{$repo_path}'. ". - "Something is misconfigured. The repository's 'Local Path' should ". - "be set to some place where the daemon can check out a working ". - "copy, and should not be inside another git repository."); - } - } - - - // This is a local command, but needs credentials. - $future = $repository->getRemoteCommandFuture('fetch --all --prune'); - $future->setCWD($local_path); - $future->resolvex(); - } - -} diff --git a/src/applications/repository/daemon/gitfetch/__init__.php b/src/applications/repository/daemon/gitfetch/__init__.php deleted file mode 100644 index 59c22675a8..0000000000 --- a/src/applications/repository/daemon/gitfetch/__init__.php +++ /dev/null @@ -1,15 +0,0 @@ -execxRemoteCommand( - 'clone %s %s', - $repository->getRemoteURI(), - rtrim($local_path, '/')); - } - - protected function executeUpdate( - PhabricatorRepository $repository, - $local_path) { - - // This is a local command, but needs credentials. - $future = $repository->getRemoteCommandFuture('pull -u'); - $future->setCWD($local_path); - - try { - $future->resolvex(); - } catch (CommandException $ex) { - $err = $ex->getError(); - $stdout = $ex->getStdOut(); - - // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the behavior - // of "hg pull" to return 1 in case of a successful pull with no changes. - // This behavior has been reverted, but users who updated between Feb 1, - // 2012 and Mar 1, 2012 will have the erroring version. Do a dumb test - // against stdout to check for this possibility. - // See: https://github.com/facebook/phabricator/issues/101/ - - // NOTE: Mercurial has translated versions, which translate this error - // string. In a translated version, the string will be something else, - // like "aucun changement trouve". There didn't seem to be an easy way - // to handle this (there are hard ways but this is not a common problem - // and only creates log spam, not application failures). Assume English. - - // TODO: Remove this once we're far enough in the future that deployment - // of 2.1 is exceedingly rare? - if ($err == 1 && preg_match('/no changes found/', $stdout)) { - return; - } else { - throw $ex; - } - } - - } - -} diff --git a/src/applications/repository/daemon/mercurialpull/__init__.php b/src/applications/repository/daemon/mercurialpull/__init__.php deleted file mode 100644 index e2b6f2382d..0000000000 --- a/src/applications/repository/daemon/mercurialpull/__init__.php +++ /dev/null @@ -1,13 +0,0 @@ -loadRepository(); - $expected_type = $this->getSupportedRepositoryType(); +/* -( Pulling Repositories )----------------------------------------------- */ - $repo_type = $repository->getVersionControlSystem(); - if ($repo_type != $expected_type) { - $repo_type_name = PhabricatorRepositoryType::getNameForRepositoryType( - $repo_type); - $expected_type_name = PhabricatorRepositoryType::getNameForRepositoryType( - $expected_type); - $repo_name = $repository->getName().' ('.$repository->getCallsign().')'; - throw new Exception( - "This daemon pulls '{$expected_type_name}' repositories, but the ". - "repository '{$repo_name}' is a '{$repo_type_name}' repository."); + + /** + * @task pull + */ + public function run() { + + // Each repository has an individual pull frequency; after we pull it, + // wait that long to pull it again. When we start up, try to pull everything + // serially. + $retry_after = array(); + + while (true) { + $repositories = $this->loadRepositories(); + + // Shuffle the repositories, then re-key the array since shuffle() + // discards keys. This is mostly for startup, we'll use soft priorities + // later. + shuffle($repositories); + $repositories = mpull($repositories, null, 'getID'); + + // If any repositories were deleted, remove them from the retry timer map + // so we don't end up with a retry timer that never gets updated and + // causes us to sleep for the minimum amount of time. + $retry_after = array_select_keys( + $retry_after, + array_keys($repositories)); + + // Assign soft priorities to repositories based on how frequently they + // should pull again. + asort($retry_after); + $repositories = array_select_keys( + $repositories, + array_keys($retry_after)) + $repositories; + + foreach ($repositories as $id => $repository) { + $after = idx($retry_after, $id, 0); + if ($after >= time()) { + continue; + } + + try { + self::pullRepository($repository); + $sleep_for = $repository->getDetail('pull-frequency', 15); + $retry_after[$id] = time() + $sleep_for; + } catch (Exception $ex) { + $retry_after[$id] = time() + 15; + phlog($ex); + } + } + + $sleep_until = max(min($retry_after), time() + 15); + sleep($sleep_until - time()); } + } + + /** + * @task pull + */ + protected function loadRepositories() { + $argv = $this->getArgv(); + if (!count($argv)) { + return id(new PhabricatorRepository())->loadAll(); + } else { + return PhabricatorRepository::loadAllByPHIDOrCallsign($argv); + } + } + + + /** + * @task pull + */ + public static function pullRepository(PhabricatorRepository $repository) { $tracked = $repository->isTracked(); if (!$tracked) { - throw new Exception("Tracking is not enabled for this repository."); + return; + } + + $vcs = $repository->getVersionControlSystem(); + + $is_svn = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_SVN); + $is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT); + $is_hg = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL); + + if ($is_svn) { + return; + } + + $callsign = $repository->getCallsign(); + + if (!$is_git && !$is_hg) { + throw new Exception( + "Unknown VCS '{$vcs}' for repository '{$callsign}'!"); } $local_path = $repository->getDetail('local-path'); - if (!$local_path) { - throw new Exception("No local path is available for this repository."); + throw new Exception( + "No local path is available for repository '{$callsign}'."); } - while (true) { - if (!Filesystem::pathExists($local_path)) { - printf("Creating new directory %s for repo %s\n", - $local_path, $repository->getName()); - execx('mkdir -p %s', dirname($local_path)); - $this->executeCreate($repository, $local_path); - } else { - $this->executeUpdate($repository, $local_path); + if (!Filesystem::pathExists($local_path)) { + $dirname = dirname($local_path); + if (!Filesystem::pathExists($dirname)) { + echo "Creating new directory '{$dirname}' ". + "for repository '{$callsign}'.\n"; + Filesystem::createDirectory($dirname, 0755, $recursive = true); + } + + if ($is_git) { + self::executeGitCreate($repository, $local_path); + } else if ($is_hg) { + self::executeHgCreate($repository, $local_path); + } + } else { + if ($is_git) { + self::executeGitUpdate($repository, $local_path); + } else if ($is_hg) { + self::executeHgUpdate($repository, $local_path); + } + } + } + + +/* -( Git Implementation )------------------------------------------------- */ + + + /** + * @task git + */ + private static function executeGitCreate( + PhabricatorRepository $repository, + $path) { + + $repository->execxRemoteCommand( + 'clone --origin origin %s %s', + $repository->getRemoteURI(), + rtrim($path, '/')); + } + + + /** + * @task git + */ + private static function executeGitUpdate( + PhabricatorRepository $repository, + $path) { + + // Run a bunch of sanity checks to detect people checking out repositories + // inside other repositories, making empty directories, pointing the local + // path at some random file or path, etc. + + list($err, $stdout) = $repository->execLocalCommand( + 'rev-parse --show-toplevel'); + + if ($err) { + + // Try to raise a more tailored error message in the more common case + // of the user creating an empty directory. (We could try to remove it, + // but might not be able to, and it's much simpler to raise a good + // message than try to navigate those waters.) + if (is_dir($path)) { + $files = Filesystem::listDirectory($path, $include_hidden = true); + if (!$files) { + throw new Exception( + "Expected to find a git repository at '{$path}', but there ". + "is an empty directory there. Remove the directory: the daemon ". + "will run 'git clone' for you."); + } + } + + throw new Exception( + "Expected to find a git repository at '{$path}', but there is ". + "a non-repository directory (with other stuff in it) there. Move or ". + "remove this directory (or reconfigure the repository to use a ". + "different directory), and then either clone a repository yourself ". + "or let the daemon do it."); + } else { + $repo_path = rtrim($stdout, "\n"); + + if (empty($repo_path)) { + throw new Exception( + "Expected to find a git repository at '{$path}', but ". + "there was no result from `git rev-parse --show-toplevel`. ". + "Something is misconfigured or broken. The git repository ". + "may be inside a '.git/' directory."); + } + + if (!Filesystem::pathsAreEquivalent($repo_path, $path)) { + throw new Exception( + "Expected to find repo at '{$path}', but the actual ". + "git repository root for this directory is '{$repo_path}'. ". + "Something is misconfigured. The repository's 'Local Path' should ". + "be set to some place where the daemon can check out a working ". + "copy, and should not be inside another git repository."); + } + } + + + // This is a local command, but needs credentials. + $future = $repository->getRemoteCommandFuture('fetch --all --prune'); + $future->setCWD($path); + $future->resolvex(); + } + + +/* -( Mercurial Implementation )------------------------------------------- */ + + + /** + * @task hg + */ + private static function executeHgCreate( + PhabricatorRepository $repository, + $path) { + + $repository->execxRemoteCommand( + 'clone %s %s', + $repository->getRemoteURI(), + rtrim($path, '/')); + } + + + /** + * @task hg + */ + private static function executeHgUpdate( + PhabricatorRepository $repository, + $path) { + + // This is a local command, but needs credentials. + $future = $repository->getRemoteCommandFuture('pull -u'); + $future->setCWD($path); + + try { + $future->resolvex(); + } catch (CommandException $ex) { + $err = $ex->getError(); + $stdout = $ex->getStdOut(); + + // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the behavior + // of "hg pull" to return 1 in case of a successful pull with no changes. + // This behavior has been reverted, but users who updated between Feb 1, + // 2012 and Mar 1, 2012 will have the erroring version. Do a dumb test + // against stdout to check for this possibility. + // See: https://github.com/facebook/phabricator/issues/101/ + + // NOTE: Mercurial has translated versions, which translate this error + // string. In a translated version, the string will be something else, + // like "aucun changement trouve". There didn't seem to be an easy way + // to handle this (there are hard ways but this is not a common problem + // and only creates log spam, not application failures). Assume English. + + // TODO: Remove this once we're far enough in the future that deployment + // of 2.1 is exceedingly rare? + if ($err == 1 && preg_match('/no changes found/', $stdout)) { + return; + } else { + throw $ex; } - $this->sleep($repository->getDetail('pull-frequency', 15)); } } diff --git a/src/applications/repository/daemon/pulllocal/__init__.php b/src/applications/repository/daemon/pulllocal/__init__.php index 14b9e230c3..cb9709df52 100644 --- a/src/applications/repository/daemon/pulllocal/__init__.php +++ b/src/applications/repository/daemon/pulllocal/__init__.php @@ -7,10 +7,12 @@ phutil_require_module('phabricator', 'applications/repository/constants/repositorytype'); -phutil_require_module('phabricator', 'applications/repository/daemon/base'); +phutil_require_module('phabricator', 'applications/repository/storage/repository'); +phutil_require_module('phabricator', 'infrastructure/daemon/base'); +phutil_require_module('phutil', 'error'); phutil_require_module('phutil', 'filesystem'); -phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'utils'); phutil_require_source('PhabricatorRepositoryPullLocalDaemon.php'); diff --git a/src/applications/repository/storage/repository/PhabricatorRepository.php b/src/applications/repository/storage/repository/PhabricatorRepository.php index 9659740836..0776c819f4 100644 --- a/src/applications/repository/storage/repository/PhabricatorRepository.php +++ b/src/applications/repository/storage/repository/PhabricatorRepository.php @@ -382,4 +382,20 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO { return 'r'.$this->getCallsign().$short_identifier; } + public static function loadAllByPHIDOrCallsign(array $names) { + $repositories = array(); + foreach ($names as $name) { + $repo = id(new PhabricatorRepository())->loadOneWhere( + 'phid = %s OR callsign = %s', + $name, + $name); + if (!$repo) { + throw new Exception( + "No repository with PHID or callsign '{$name}' exists!"); + } + $repositories[$repo->getID()] = $repo; + } + return $repositories; + } + }