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

scripts/sql/manage_storage.php

@@ -90,7 +90,9 @@ if (strlen($host)) {
   // 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,89 +12,6 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
   }
 
   protected function executeChecks() {
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
-    if (!$master) {
-      // If we're implicitly in read-only mode during disaster recovery,
-      // don't bother with these setup checks.
-      return;
-    }
-
-    $conn_raw = $master->newManagementConnection();
-
-    try {
-      queryfx($conn_raw, 'SELECT 1');
-      $database_exception = null;
-    } catch (AphrontInvalidCredentialsQueryException $ex) {
-      $database_exception = $ex;
-    } catch (AphrontConnectionQueryException $ex) {
-      $database_exception = $ex;
-    }
-
-    if ($database_exception) {
-      $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue(
-        $database_exception);
-      $this->addIssue($issue);
-      return;
-    }
-
-    $engines = queryfx_all($conn_raw, 'SHOW ENGINES');
-    $engines = ipull($engines, 'Support', 'Engine');
-
-    $innodb = idx($engines, 'InnoDB');
-    if ($innodb != 'YES' && $innodb != 'DEFAULT') {
-      $message = pht(
-        "The 'InnoDB' engine is not available in MySQL. Enable InnoDB in ".
-        "your MySQL configuration.".
-        "\n\n".
-        "(If you aleady created tables, MySQL incorrectly used some other ".
-        "engine to create them. You need to convert them or drop and ".
-        "reinitialize them.)");
-
-      $this->newIssue('mysql.innodb')
-        ->setName(pht('MySQL InnoDB Engine Not Available'))
-        ->setMessage($message)
-        ->setIsFatal(true);
-      return;
-    }
-
-    $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace');
-
-    $databases = queryfx_all($conn_raw, 'SHOW DATABASES');
-    $databases = ipull($databases, 'Database', 'Database');
-
-    if (empty($databases[$namespace.'_meta_data'])) {
-      $message = pht(
-        "Run the storage upgrade script to setup Phabricator's database ".
-        "schema.");
-
-      $this->newIssue('storage.upgrade')
-        ->setName(pht('Setup MySQL Schema'))
-        ->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'));
-      }
-    }
-
     $host = PhabricatorEnv::getEnvConfig('mysql.host');
     $matches = null;
     if (preg_match('/^([^:]+):(\d+)$/', $host, $matches)) {
@@ -126,5 +43,97 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
             $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 {
+      queryfx($conn_raw, 'SELECT 1');
+      $database_exception = null;
+    } catch (AphrontInvalidCredentialsQueryException $ex) {
+      $database_exception = $ex;
+    } catch (AphrontConnectionQueryException $ex) {
+      $database_exception = $ex;
+    }
+
+    if ($database_exception) {
+      $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue(
+        $database_exception);
+      $this->addIssue($issue);
+      return true;
+    }
+
+    $engines = queryfx_all($conn_raw, 'SHOW ENGINES');
+    $engines = ipull($engines, 'Support', 'Engine');
+
+    $innodb = idx($engines, 'InnoDB');
+    if ($innodb != 'YES' && $innodb != 'DEFAULT') {
+      $message = pht(
+        "The 'InnoDB' engine is not available in MySQL. Enable InnoDB in ".
+        "your MySQL configuration.".
+        "\n\n".
+        "(If you aleady created tables, MySQL incorrectly used some other ".
+        "engine to create them. You need to convert them or drop and ".
+        "reinitialize them.)");
+
+      $this->newIssue('mysql.innodb')
+        ->setName(pht('MySQL InnoDB Engine Not Available'))
+        ->setMessage($message)
+        ->setIsFatal(true);
+      return true;
+    }
+
+    $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace');
+
+    $databases = queryfx_all($conn_raw, 'SHOW DATABASES');
+    $databases = ipull($databases, 'Database', 'Database');
+
+    if (empty($databases[$namespace.'_meta_data'])) {
+      $message = pht(
+        "Run the storage upgrade script to setup Phabricator's database ".
+        "schema.");
+
+      $this->newIssue('storage.upgrade')
+        ->setName(pht('Setup MySQL Schema'))
+        ->setMessage($message)
+        ->setIsFatal(true)
+        ->addCommand(hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
+      return true;
+    }
+
+    $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;
+    }
   }
 }

src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php

@@ -85,14 +85,6 @@ final class PhabricatorClusterDatabasesConfigOptionType
       $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 @@ final class PhabricatorDatabaseRef
     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,18 +495,14 @@ final class PhabricatorDatabaseRef
       ->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
+    $replicas = array();
-    // 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.
-
     foreach ($refs as $ref) {
       if ($ref->getDisabled()) {
         continue;
@@ -499,10 +510,24 @@ final class PhabricatorDatabaseRef
       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 @@ final class PhabricatorEnv extends Phobject {
       $stack->pushSource($site_source);
     }
 
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
+    $masters = PhabricatorDatabaseRef::getMasterDatabaseRefs();
-    if (!$master) {
+    if (!$masters) {
       self::setReadOnly(true, self::READONLY_MASTERLESS);
-    } else if ($master->isSevered()) {
+    } else {
-      $master->checkHealth();
+      // If any master is severed, we drop to readonly mode. In theory we
-      if ($master->isSevered()) {
+      // could try to continue if we're only missing some applications, but
-        self::setReadOnly(true, self::READONLY_SEVERED);
+      // 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 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
   }
 
   private function newClusterConnection($database, $mode) {
-    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
+    $master = PhabricatorDatabaseRef::getMasterDatabaseRefForDatabase(
+      $database);
 
     if ($master && !$master->isSevered()) {
       $connection = $master->newApplicationConnection($database);
@@ -130,7 +131,8 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
       }
     }
 
-    $replica = PhabricatorDatabaseRef::getReplicaDatabaseRef();
+    $replica = PhabricatorDatabaseRef::getReplicaDatabaseRefForDatabase(
+      $database);
     if ($replica) {
       $connection = $replica->newApplicationConnection($database);
       $connection->setReadOnly(true);