Support multiple database masters and convert easy callers

Summary:
Ref T11044. This moves toward partitioned application databases:

  - You can define multiple masters.
  - Convert all the easily-convertible code to become multi-master aware.

This doesn't convert most of `bin/storage` or "Config > Database (Stuff)" yet, as both are quite involved. They still work for now, but only operate on the first master instead of all masters.

Test Plan: Configured multiple masters, browsed around, ran `bin/storage` commands, ran `bin/storage --host ...`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11044

Differential Revision: https://secure.phabricator.com/D16115

Author: epriestley

scripts/sql/manage_storage.php

@@ -90,7 +90,9 @@
   // Include the master in case the user is just specifying a redundant
   // "--host" flag for no reason and does not actually have a database
   // cluster configured.
-  $refs[] = PhabricatorDatabaseRef::getMasterDatabaseRef();
+  foreach (PhabricatorDatabaseRef::getMasterDatabaseRefs() as $master_ref) {
+    $refs[] = $master_ref;
+  }
 
   foreach ($refs as $possible_ref) {
     if ($possible_ref->getHost() == $host) {

src/applications/config/check/PhabricatorDatabaseSetupCheck.php

@@ -12,13 +12,52 @@ public function getExecutionOrder() {
   }
 
   protected function executeChecks() {
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
-    if (!$master) {
+    $host = PhabricatorEnv::getEnvConfig('mysql.host');
+    $matches = null;
+    if (preg_match('/^([^:]+):(\d+)$/', $host, $matches)) {
+      $host = $matches[1];
+      $port = $matches[2];
+
+      $this->newIssue('storage.mysql.hostport')
+        ->setName(pht('Deprecated mysql.host Format'))
+        ->setSummary(
+          pht(
+            'Move port information from `%s` to `%s` in your config.',
+            'mysql.host',
+            'mysql.port'))
+        ->setMessage(
+          pht(
+            'Your `%s` configuration contains a port number, but this usage '.
+            'is deprecated. Instead, put the port number in `%s`.',
+            'mysql.host',
+            'mysql.port'))
+        ->addPhabricatorConfig('mysql.host')
+        ->addPhabricatorConfig('mysql.port')
+        ->addCommand(
+          hsprintf(
+            '<tt>phabricator/ $</tt> ./bin/config set mysql.host %s',
+            $host))
+        ->addCommand(
+          hsprintf(
+            '<tt>phabricator/ $</tt> ./bin/config set mysql.port %s',
+            $port));
+    }
+
+    $masters = PhabricatorDatabaseRef::getMasterDatabaseRefs();
+    if (!$masters) {
       // If we're implicitly in read-only mode during disaster recovery,
       // don't bother with these setup checks.
       return;
     }
 
+    foreach ($masters as $master) {
+      if ($this->checkMasterDatabase($master)) {
+        break;
+      }
+    }
+  }
+
+  private function checkMasterDatabase(PhabricatorDatabaseRef $master) {
     $conn_raw = $master->newManagementConnection();
 
     try {
@@ -34,7 +73,7 @@ protected function executeChecks() {
       $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue(
         $database_exception);
       $this->addIssue($issue);
-      return;
+      return true;
     }
 
     $engines = queryfx_all($conn_raw, 'SHOW ENGINES');
@@ -54,7 +93,7 @@ protected function executeChecks() {
         ->setName(pht('MySQL InnoDB Engine Not Available'))
         ->setMessage($message)
         ->setIsFatal(true);
-      return;
+      return true;
     }
 
     $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace');
@@ -72,59 +111,29 @@ protected function executeChecks() {
         ->setMessage($message)
         ->setIsFatal(true)
         ->addCommand(hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
-    } else {
-      $conn_meta = $master->newApplicationConnection(
-        $namespace.'_meta_data');
-
-      $applied = queryfx_all($conn_meta, 'SELECT patch FROM patch_status');
-      $applied = ipull($applied, 'patch', 'patch');
-
-      $all = PhabricatorSQLPatchList::buildAllPatches();
-      $diff = array_diff_key($all, $applied);
-
-      if ($diff) {
-        $this->newIssue('storage.patch')
-          ->setName(pht('Upgrade MySQL Schema'))
-          ->setMessage(
-            pht(
-              "Run the storage upgrade script to upgrade Phabricator's ".
-              "database schema. Missing patches:<br />%s<br />",
-              phutil_implode_html(phutil_tag('br'), array_keys($diff))))
-          ->addCommand(
-            hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
-      }
+      return true;
     }
 
-    $host = PhabricatorEnv::getEnvConfig('mysql.host');
-    $matches = null;
-    if (preg_match('/^([^:]+):(\d+)$/', $host, $matches)) {
-      $host = $matches[1];
-      $port = $matches[2];
+    $conn_meta = $master->newApplicationConnection(
+      $namespace.'_meta_data');
 
-      $this->newIssue('storage.mysql.hostport')
-        ->setName(pht('Deprecated mysql.host Format'))
-        ->setSummary(
-          pht(
-            'Move port information from `%s` to `%s` in your config.',
-            'mysql.host',
-            'mysql.port'))
+    $applied = queryfx_all($conn_meta, 'SELECT patch FROM patch_status');
+    $applied = ipull($applied, 'patch', 'patch');
+
+    $all = PhabricatorSQLPatchList::buildAllPatches();
+    $diff = array_diff_key($all, $applied);
+
+    if ($diff) {
+      $this->newIssue('storage.patch')
+        ->setName(pht('Upgrade MySQL Schema'))
         ->setMessage(
           pht(
-            'Your `%s` configuration contains a port number, but this usage '.
-            'is deprecated. Instead, put the port number in `%s`.',
-            'mysql.host',
-            'mysql.port'))
-        ->addPhabricatorConfig('mysql.host')
-        ->addPhabricatorConfig('mysql.port')
+            "Run the storage upgrade script to upgrade Phabricator's ".
+            "database schema. Missing patches:<br />%s<br />",
+            phutil_implode_html(phutil_tag('br'), array_keys($diff))))
         ->addCommand(
-          hsprintf(
-            '<tt>phabricator/ $</tt> ./bin/config set mysql.host %s',
-            $host))
-        ->addCommand(
-          hsprintf(
-            '<tt>phabricator/ $</tt> ./bin/config set mysql.port %s',
-            $port));
+          hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
+      return true;
     }
-
   }
 }

src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php

@@ -85,14 +85,6 @@ public function validateOption(PhabricatorConfigOption $option, $value) {
       $map[$key] = true;
     }
 
-    if (count($masters) > 1) {
-      throw new Exception(
-        pht(
-          'Database cluster configuration is invalid: it describes multiple '.
-          'masters. No more than one host may be a master. Hosts currently '.
-          'configured as masters: %s.',
-          implode(', ', $masters)));
-    }
   }
 
 }

src/infrastructure/cluster/PhabricatorDatabaseRef.php

@@ -446,24 +446,39 @@ public function getHealthRecord() {
     return $this->healthRecord;
   }
 
-  public static function getMasterDatabaseRef() {
+  public static function getMasterDatabaseRefs() {
     $refs = self::getLiveRefs();
 
     if (!$refs) {
-      return self::getLiveIndividualRef();
+      return array(self::getLiveIndividualRef());
     }
 
-    $master = null;
+    $masters = array();
     foreach ($refs as $ref) {
       if ($ref->getDisabled()) {
         continue;
       }
       if ($ref->getIsMaster()) {
-        return $ref;
+        $masters[] = $ref;
       }
     }
 
-    return null;
+    return $masters;
+  }
+
+  public static function getMasterDatabaseRef() {
+    // TODO: Remove this method; it no longer makes sense with application
+    // partitioning.
+
+    return head(self::getMasterDatabaseRefs());
+  }
+
+  public static function getMasterDatabaseRefForDatabase($database) {
+    $masters = self::getMasterDatabaseRefs();
+
+    // TODO: Actually implement this.
+
+    return head($masters);
   }
 
   public static function newIndividualRef() {
@@ -480,29 +495,39 @@ public static function newIndividualRef() {
       ->setIsMaster(true);
   }
 
-  public static function getReplicaDatabaseRef() {
+  public static function getReplicaDatabaseRefs() {
     $refs = self::getLiveRefs();
 
     if (!$refs) {
-      return null;
+      return array();
     }
 
-    // TODO: We may have multiple replicas to choose from, and could make
-    // more of an effort to pick the "best" one here instead of always
-    // picking the first one. Once we've picked one, we should try to use
-    // the same replica for the rest of the request, though.
-
+    $replicas = array();
     foreach ($refs as $ref) {
       if ($ref->getDisabled()) {
         continue;
       }
       if ($ref->getIsMaster()) {
         continue;
       }
-      return $ref;
+
+      $replicas[] = $ref;
     }
 
-    return null;
+    return $replicas;
+  }
+
+  public static function getReplicaDatabaseRefForDatabase($database) {
+    $replicas = self::getReplicaDatabaseRefs();
+
+    // TODO: Actually implement this.
+
+    // TODO: We may have multiple replicas to choose from, and could make
+    // more of an effort to pick the "best" one here instead of always
+    // picking the first one. Once we've picked one, we should try to use
+    // the same replica for the rest of the request, though.
+
+    return head($replicas);
   }
 
   private function newConnection(array $options) {

src/infrastructure/env/PhabricatorEnv.php

@@ -223,13 +223,24 @@ private static function buildConfigurationSourceStack($config_optional) {
       $stack->pushSource($site_source);
     }
 
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
-    if (!$master) {
+    $masters = PhabricatorDatabaseRef::getMasterDatabaseRefs();
+    if (!$masters) {
       self::setReadOnly(true, self::READONLY_MASTERLESS);
-    } else if ($master->isSevered()) {
-      $master->checkHealth();
-      if ($master->isSevered()) {
-        self::setReadOnly(true, self::READONLY_SEVERED);
+    } else {
+      // If any master is severed, we drop to readonly mode. In theory we
+      // could try to continue if we're only missing some applications, but
+      // this is very complex and we're unlikely to get it right.
+
+      foreach ($masters as $master) {
+        // Give severed masters one last chance to get healthy.
+        if ($master->isSevered()) {
+          $master->checkHealth();
+        }
+
+        if ($master->isSevered()) {
+          self::setReadOnly(true, self::READONLY_SEVERED);
+          break;
+        }
       }
     }
 

src/infrastructure/storage/lisk/PhabricatorLiskDAO.php

@@ -114,7 +114,8 @@ private function newBasicConnection($database, $mode, $namespace) {
   }
 
   private function newClusterConnection($database, $mode) {
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
+    $master = PhabricatorDatabaseRef::getMasterDatabaseRefForDatabase(
+      $database);
 
     if ($master && !$master->isSevered()) {
       $connection = $master->newApplicationConnection($database);
@@ -130,7 +131,8 @@ private function newClusterConnection($database, $mode) {
       }
     }
 
-    $replica = PhabricatorDatabaseRef::getReplicaDatabaseRef();
+    $replica = PhabricatorDatabaseRef::getReplicaDatabaseRefForDatabase(
+      $database);
     if ($replica) {
       $connection = $replica->newApplicationConnection($database);
       $connection->setReadOnly(true);