From 0f144d29e92059b787cfb3816a0d3988d4b1feb3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 2 Apr 2017 09:27:19 -0700
Subject: [PATCH] When "cluster.search" changes, don't trust the old index
 versions

Summary:
Ref T12450. We track a "document version" for updating search indexes, so that if a document is rapidly updated many times in a row we can skip most of the work.

However, this version doesn't consider "cluster.search" configuration, so if you add a new service (like a new ElasticSearch host) we still think that every document is up-to-date. When you run `bin/search index` to populate the index (without `--force`), we just do nothing.

This isn't necessarily very obvious. D17597 makes it more clear, by printing "everything was skipped and nothing happened" at the end.

Here, fix the issue by considering the content of "cluster.search" when computing fulltext document versions: if you change `cluster.search`, we throw away the version index and reindex everything.

This is slightly more work than we need to do, but changes to "cluster.search" are rare and this is much easier than trying to individually track which versions of which documents are in which services, which probably isn't very useful anyway.

Test Plan:
  - Ran `bin/search index --type project`, saw everything get skipped.
  - Changed `cluster.search`.
  - Ran `search index` again, saw everything get updated.
  - Ran a third time without changing `cluster.search`, everything was properly skipped.

Reviewers: chad, 20after4

Reviewed By: 20after4

Maniphest Tasks: T12450

Differential Revision: https://secure.phabricator.com/D17598
---
 ...habricatorFulltextIndexEngineExtension.php | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php
index 0767849abe..ab4da88420 100644
--- a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php
+++ b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php
@@ -5,6 +5,8 @@ final class PhabricatorFulltextIndexEngineExtension
 
   const EXTENSIONKEY = 'fulltext';
 
+  private $configurationVersion;
+
   public function getExtensionName() {
     return pht('Fulltext Engine');
   }
@@ -12,6 +14,11 @@ final class PhabricatorFulltextIndexEngineExtension
   public function getIndexVersion($object) {
     $version = array();
 
+    // When "cluster.search" is reconfigured, new indexes which don't have any
+    // data yet may have been added. We err on the side of caution and assume
+    // that every document may need to be reindexed.
+    $version[] = $this->getConfigurationVersion();
+
     if ($object instanceof PhabricatorApplicationTransactionInterface) {
       // If this is a normal object with transactions, we only need to
       // reindex it if there are new transactions (or comment edits).
@@ -88,5 +95,22 @@ final class PhabricatorFulltextIndexEngineExtension
     return $comment_row['id'];
   }
 
+  private function getConfigurationVersion() {
+    if ($this->configurationVersion === null) {
+      $this->configurationVersion = $this->newConfigurationVersion();
+    }
+    return $this->configurationVersion;
+  }
+
+  private function newConfigurationVersion() {
+    $raw = array(
+      'services' => PhabricatorEnv::getEnvConfig('cluster.search'),
+    );
+
+    $json = phutil_json_encode($raw);
+
+    return PhabricatorHash::digestForIndex($json);
+  }
+
 
 }