From 304d19f92a7bea08573045d6951cefa4b14e7086 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 2 Apr 2017 09:53:26 -0700
Subject: [PATCH] After a fulltext write to a particular service fails, keep
 trying writes to other services

Summary:
Ref T12450. Currently, if a write fails, we stop and don't try to write to other index services. There's no technical reason not to keep trying writes, it makes some testing easier, and it would improve behavior in a scenario where engines are configured as "primary" and "backup" and the primary service is having some issues.

Also, make "no writable services are configured" acceptable, rather than an error. This state is probably goofy but if we want to detect it I think it should probably be a config-validation issue, not a write-time check. I also think it's not totally unreasonable to want to just turn off all writes for a while (maybe to reduce load while you're doing a background update).

Test Plan:
  - Configured a bad ElasticSearch engine and a good MySQL engine.
  - Ran `bin/search index ... --force`.
  - Saw MySQL get updated even though ElasticSearch failed.

Reviewers: chad, 20after4

Reviewed By: 20after4

Maniphest Tasks: T12450

Differential Revision: https://secure.phabricator.com/D17599
---
 .../search/PhabricatorSearchService.php       | 28 +++++++++++++------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/infrastructure/cluster/search/PhabricatorSearchService.php b/src/infrastructure/cluster/search/PhabricatorSearchService.php
index 10cf78d94b..a9ceb0e7e5 100644
--- a/src/infrastructure/cluster/search/PhabricatorSearchService.php
+++ b/src/infrastructure/cluster/search/PhabricatorSearchService.php
@@ -212,20 +212,30 @@ class PhabricatorSearchService
   /**
    * (re)index the document: attempt to pass the document to all writable
    * fulltext search hosts
-   * @throws PhabricatorClusterNoHostForRoleException
    */
   public static function reindexAbstractDocument(
-    PhabricatorSearchAbstractDocument $doc) {
-    $indexed = 0;
+    PhabricatorSearchAbstractDocument $document) {
+
+    $exceptions = array();
     foreach (self::getAllServices() as $service) {
-      $hosts = $service->getAllHostsForRole('write');
-      if (count($hosts)) {
-        $service->getEngine()->reindexAbstractDocument($doc);
-        $indexed++;
+      if (!$service->isWritable()) {
+        continue;
+      }
+
+      $engine = $service->getEngine();
+      try {
+        $engine->reindexAbstractDocument($document);
+      } catch (Exception $ex) {
+        $exceptions[] = $ex;
       }
     }
-    if ($indexed == 0) {
-      throw new PhabricatorClusterNoHostForRoleException('write');
+
+    if ($exceptions) {
+      throw new PhutilAggregateException(
+        pht(
+          'Writes to search services failed while reindexing document "%s".',
+          $document->getPHID()),
+        $exceptions);
     }
   }