From 29a3cd5121d9869f8fc7d5e9a07ade4dcff1c735 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 30 Nov 2016 10:26:32 -0800 Subject: [PATCH] Add "Manual Activities", to tell administrators to rebuild the search index Summary: Ref T11922. After updating to HEAD of `master`, you need to manually rebuild the index. We don't do this during `bin/storage upgrade` because it can take a very long time (`secure.phabricator.com` took roughly an hour) and can happen while Phabricator is running. However, if we don't warn users about this they'll just get a broken index unless they go read the changelog (or file an issue, then we tell them to go read the changelog). This adds a very simple table for notes to administrators so we can write a "you need to go rebuild the index" note, then adds one. Administrators clear the note by completing the activity and running `bin/config done reindex`. This isn't automatic because there are various strategies you can use to approach the issue, which I'll discuss in greater detail in the linked documentation. Also, fix an issue where `bin/storage upgrade --apply ` could try to re-mark an already-applied patch as applied. Test Plan: - Ran storage ugrades. - Got instructions to rebuild search index. - Cleared instructions with `bin/config done reindex`. Reviewers: chad Reviewed By: chad Subscribers: avivey Maniphest Tasks: T11922 Differential Revision: https://secure.phabricator.com/D16965 --- .../autopatches/20161130.search.01.manual.sql | 6 ++ .../20161130.search.02.rebuild.php | 26 +++++++ src/__phutil_library_map__.php | 6 ++ .../PhabricatorManualActivitySetupCheck.php | 75 +++++++++++++++++++ ...habricatorConfigManagementDoneWorkflow.php | 54 +++++++++++++ .../PhabricatorConfigManualActivity.php | 38 ++++++++++ .../PhabricatorStorageManagementWorkflow.php | 4 +- 7 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 resources/sql/autopatches/20161130.search.01.manual.sql create mode 100644 resources/sql/autopatches/20161130.search.02.rebuild.php create mode 100644 src/applications/config/check/PhabricatorManualActivitySetupCheck.php create mode 100644 src/applications/config/management/PhabricatorConfigManagementDoneWorkflow.php create mode 100644 src/applications/config/storage/PhabricatorConfigManualActivity.php diff --git a/resources/sql/autopatches/20161130.search.01.manual.sql b/resources/sql/autopatches/20161130.search.01.manual.sql new file mode 100644 index 0000000000..a6928a782d --- /dev/null +++ b/resources/sql/autopatches/20161130.search.01.manual.sql @@ -0,0 +1,6 @@ +CREATE TABLE {$NAMESPACE}_config.config_manualactivity ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + activityType VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT}, + parameters LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}, + UNIQUE KEY `key_type` (activityType) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20161130.search.02.rebuild.php b/resources/sql/autopatches/20161130.search.02.rebuild.php new file mode 100644 index 0000000000..a5a9755839 --- /dev/null +++ b/resources/sql/autopatches/20161130.search.02.rebuild.php @@ -0,0 +1,26 @@ +establishConnection('r'); + + // We're only going to require this if the index isn't empty: if you're on a + // fresh install, you don't have to do anything. + $any_documents = queryfx_one( + $conn, + 'SELECT * FROM %T LIMIT 1', + $field->getTableName()); + + if ($any_documents) { + try { + id(new PhabricatorConfigManualActivity()) + ->setActivityType(PhabricatorConfigManualActivity::TYPE_REINDEX) + ->save(); + } catch (AphrontDuplicateKeyQueryException $ex) { + // If we've already noted that this activity is required, just move on. + } + } +} diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b682a9809a..e2cdbb99ea 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2272,11 +2272,13 @@ phutil_register_library_map(array( 'PhabricatorConfigListController' => 'applications/config/controller/PhabricatorConfigListController.php', 'PhabricatorConfigLocalSource' => 'infrastructure/env/PhabricatorConfigLocalSource.php', 'PhabricatorConfigManagementDeleteWorkflow' => 'applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php', + 'PhabricatorConfigManagementDoneWorkflow' => 'applications/config/management/PhabricatorConfigManagementDoneWorkflow.php', 'PhabricatorConfigManagementGetWorkflow' => 'applications/config/management/PhabricatorConfigManagementGetWorkflow.php', 'PhabricatorConfigManagementListWorkflow' => 'applications/config/management/PhabricatorConfigManagementListWorkflow.php', 'PhabricatorConfigManagementMigrateWorkflow' => 'applications/config/management/PhabricatorConfigManagementMigrateWorkflow.php', 'PhabricatorConfigManagementSetWorkflow' => 'applications/config/management/PhabricatorConfigManagementSetWorkflow.php', 'PhabricatorConfigManagementWorkflow' => 'applications/config/management/PhabricatorConfigManagementWorkflow.php', + 'PhabricatorConfigManualActivity' => 'applications/config/storage/PhabricatorConfigManualActivity.php', 'PhabricatorConfigModule' => 'applications/config/module/PhabricatorConfigModule.php', 'PhabricatorConfigModuleController' => 'applications/config/controller/PhabricatorConfigModuleController.php', 'PhabricatorConfigOption' => 'applications/config/option/PhabricatorConfigOption.php', @@ -2896,6 +2898,7 @@ phutil_register_library_map(array( 'PhabricatorManiphestApplication' => 'applications/maniphest/application/PhabricatorManiphestApplication.php', 'PhabricatorManiphestConfigOptions' => 'applications/maniphest/config/PhabricatorManiphestConfigOptions.php', 'PhabricatorManiphestTaskTestDataGenerator' => 'applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php', + 'PhabricatorManualActivitySetupCheck' => 'applications/config/check/PhabricatorManualActivitySetupCheck.php', 'PhabricatorMarkupCache' => 'applications/cache/storage/PhabricatorMarkupCache.php', 'PhabricatorMarkupEngine' => 'infrastructure/markup/PhabricatorMarkupEngine.php', 'PhabricatorMarkupEngineTestCase' => 'infrastructure/markup/__tests__/PhabricatorMarkupEngineTestCase.php', @@ -7178,11 +7181,13 @@ phutil_register_library_map(array( 'PhabricatorConfigListController' => 'PhabricatorConfigController', 'PhabricatorConfigLocalSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigManagementDeleteWorkflow' => 'PhabricatorConfigManagementWorkflow', + 'PhabricatorConfigManagementDoneWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementGetWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementListWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementMigrateWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementSetWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'PhabricatorConfigManualActivity' => 'PhabricatorConfigEntryDAO', 'PhabricatorConfigModule' => 'Phobject', 'PhabricatorConfigModuleController' => 'PhabricatorConfigController', 'PhabricatorConfigOption' => array( @@ -7877,6 +7882,7 @@ phutil_register_library_map(array( 'PhabricatorManiphestApplication' => 'PhabricatorApplication', 'PhabricatorManiphestConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorManiphestTaskTestDataGenerator' => 'PhabricatorTestDataGenerator', + 'PhabricatorManualActivitySetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorMarkupCache' => 'PhabricatorCacheDAO', 'PhabricatorMarkupEngine' => 'Phobject', 'PhabricatorMarkupEngineTestCase' => 'PhabricatorTestCase', diff --git a/src/applications/config/check/PhabricatorManualActivitySetupCheck.php b/src/applications/config/check/PhabricatorManualActivitySetupCheck.php new file mode 100644 index 0000000000..44db1b57a6 --- /dev/null +++ b/src/applications/config/check/PhabricatorManualActivitySetupCheck.php @@ -0,0 +1,75 @@ +loadAll(); + + foreach ($activities as $activity) { + $type = $activity->getActivityType(); + + // For now, there is only one type of manual activity. It's not clear + // if we're really going to have too much more of this stuff so this + // is a bit under-designed for now. + + $activity_name = pht('Rebuild Search Index'); + $activity_summary = pht( + 'The search index algorithm has been updated and the index needs '. + 'be rebuilt.'); + + $message = array(); + + $message[] = pht( + 'The indexing algorithm for the fulltext search index has been '. + 'updated and the index needs to be rebuilt. Until you rebuild the '. + 'index, global search (and other fulltext search) will not '. + 'function correctly.'); + + $message[] = pht( + 'You can rebuild the search index while Phabricator is running.'); + + $message[] = pht( + 'To rebuild the index, run this command:'); + + $message[] = phutil_tag( + 'pre', + array(), + (string)csprintf( + 'phabricator/ $ ./bin/search index --all --force --background')); + + $message[] = pht( + 'You can find more information about rebuilding the search '. + 'index here: %s', + phutil_tag( + 'a', + array( + 'href' => 'https://phurl.io/u/reindex', + 'target' => '_blank', + ), + 'https://phurl.io/u/reindex')); + + $message[] = pht( + 'After rebuilding the index, run this command to clear this setup '. + 'warning:'); + + $message[] = phutil_tag( + 'pre', + array(), + (string)csprintf('phabricator/ $ ./bin/config done %R', $type)); + + $activity_message = phutil_implode_html("\n\n", $message); + + $this->newIssue('manual.'.$type) + ->setName($activity_name) + ->setSummary($activity_summary) + ->setMessage($activity_message); + } + + } + +} diff --git a/src/applications/config/management/PhabricatorConfigManagementDoneWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementDoneWorkflow.php new file mode 100644 index 0000000000..1369e9ebc8 --- /dev/null +++ b/src/applications/config/management/PhabricatorConfigManagementDoneWorkflow.php @@ -0,0 +1,54 @@ +setName('done') + ->setExamples('**done** __activity__') + ->setSynopsis(pht('Mark a manual upgrade activity as complete.')) + ->setArguments( + array( + array( + 'name' => 'activities', + 'wildcard' => true, + ), + )); + } + + public function execute(PhutilArgumentParser $args) { + $activities = $args->getArg('activities'); + if (!$activities) { + throw new PhutilArgumentUsageException( + pht('Specify an activity to mark as completed.')); + } + + foreach ($activities as $type) { + $activity = id(new PhabricatorConfigManualActivity())->loadOneWhere( + 'activityType = %s', + $type); + if (!$activity) { + throw new PhutilArgumentUsageException( + pht( + 'Activity "%s" is not currently marked as required, so there '. + 'is no need to complete it.', + $type)); + } else { + $activity->delete(); + echo tsprintf( + "%s\n", + pht( + 'Marked activity "%s" as completed.', + $type)); + } + } + + echo tsprintf( + "%s\n", + pht('Done.')); + + return 0; + } + +} diff --git a/src/applications/config/storage/PhabricatorConfigManualActivity.php b/src/applications/config/storage/PhabricatorConfigManualActivity.php new file mode 100644 index 0000000000..44c26f77dd --- /dev/null +++ b/src/applications/config/storage/PhabricatorConfigManualActivity.php @@ -0,0 +1,38 @@ + false, + self::CONFIG_SERIALIZATION => array( + 'parameters' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'activityType' => 'text64', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_type' => array( + 'columns' => array('activityType'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + + public function setParameter($key, $value) { + $this->parameters[$key] = $value; + return $this; + } + + public function getParameter($key, $default = null) { + return idx($this->parameters, $key, $default); + } + +} diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php index caea88a608..e915cedb8d 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php @@ -929,6 +929,7 @@ abstract class PhabricatorStorageManagementWorkflow } $applied_map = array(); + $state_map = array(); foreach ($api_map as $ref_key => $api) { $applied = $api->getAppliedPatches(); @@ -946,6 +947,7 @@ abstract class PhabricatorStorageManagementWorkflow } $applied = array_fuse($applied); + $state_map[$ref_key] = $applied; if ($apply_only) { if (isset($applied[$apply_only])) { @@ -1097,7 +1099,7 @@ abstract class PhabricatorStorageManagementWorkflow // If we're explicitly reapplying this patch, we don't need to // mark it as applied. - if (!isset($applied_map[$ref_key][$key])) { + if (!isset($state_map[$ref_key][$key])) { $api->markPatchApplied($key, ($t_end - $t_begin)); $applied_map[$ref_key][$key] = true; }