When cluster.databases is configured, read the master connection from it

Summary:
Ref T4571. Ref T10759. Ref T10758. This isn't complete, but gets most of the job done:

  - When `cluster.databases` is set up, most things ignore `mysql.host` now.
  - You can `bin/storage upgrade` and stuff works.
  - You can browse around in the web UI and stuff works.

There's still a lot of weird tricky stuff to navigate, and this has real no advantages over configuring a single server yet (no automatic failover, etc).

Test Plan:
  - Configured `cluster.databases` to point at my `t1.micro` hosts in EC2 (master + replica).
  - Ran `bin/storage upgrade`, got a new install setup on them properly.
  - Survived setup warnings, browsed around.
  - Switched back to local config, ran `bin/storage upgrade`, browsed around, went through setup checks.
  - Intentionally broke config (bad hosts, no masters) and things seemed to react reasonably well.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4571, T10758, T10759

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

scripts/sql/manage_storage.php

@@ -19,13 +19,6 @@ EOHELP
 );
 $args->parseStandardArguments();
 
-$conf = PhabricatorEnv::newObjectFromConfig(
-  'mysql.configuration-provider',
-  array($dao = null, 'w'));
-
-$default_user       = $conf->getUser();
-$default_host       = $conf->getHost();
-$default_port       = $conf->getPort();
 $default_namespace  = PhabricatorLiskDAO::getDefaultStorageNamespace();
 
 try {
@@ -41,10 +34,8 @@ try {
         'name'    => 'user',
         'short'   => 'u',
         'param'   => 'username',
-        'default' => $default_user,
         'help'    => pht(
-          "Connect with __username__ instead of the configured default ('%s').",
+          'Connect with __username__ instead of the configured default.'),
-          $default_user),
       ),
       array(
         'name'    => 'password',
@@ -84,11 +75,21 @@ try {
 // First, test that the Phabricator configuration is set up correctly. After
 // we know this works we'll test any administrative credentials specifically.
 
+$ref = PhabricatorDatabaseRef::getMasterDatabaseRef();
+if (!$ref) {
+  throw new Exception(
+    pht('No database master is configured.'));
+}
+
+$default_user = $ref->getUser();
+$default_host = $ref->getHost();
+$default_port = $ref->getPort();
+
 $test_api = id(new PhabricatorStorageManagementAPI())
   ->setUser($default_user)
   ->setHost($default_host)
   ->setPort($default_port)
-  ->setPassword($conf->getPassword())
+  ->setPassword($ref->getPass())
   ->setNamespace($args->getArg('namespace'));
 
 try {
@@ -120,15 +121,20 @@ try {
 
 if ($args->getArg('password') === null) {
   // This is already a PhutilOpaqueEnvelope.
-  $password = $conf->getPassword();
+  $password = $ref->getPass();
 } else {
   // Put this in a PhutilOpaqueEnvelope.
   $password = new PhutilOpaqueEnvelope($args->getArg('password'));
   PhabricatorEnv::overrideConfig('mysql.pass', $args->getArg('password'));
 }
 
+$selected_user = $args->getArg('user');
+if ($selected_user === null) {
+  $selected_user = $default_user;
+}
+
 $api = id(new PhabricatorStorageManagementAPI())
-  ->setUser($args->getArg('user'))
+  ->setUser($selected_user)
   ->setHost($default_host)
   ->setPort($default_port)
   ->setPassword($password)

src/applications/config/check/PhabricatorDatabaseSetupCheck.php

@@ -12,25 +12,14 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
   }
 
   protected function executeChecks() {
-    $conf = PhabricatorEnv::newObjectFromConfig('mysql.configuration-provider');
+    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
-    $conn_user = $conf->getUser();
+    if (!$master) {
-    $conn_pass = $conf->getPassword();
+      // If we're implicitly in read-only mode during disaster recovery,
-    $conn_host = $conf->getHost();
+      // don't bother with these setup checks.
-    $conn_port = $conf->getPort();
+      return;
+    }
 
-    ini_set('mysql.connect_timeout', 2);
+    $conn_raw = $master->newManagementConnection();
-
-    $config = array(
-      'user'      => $conn_user,
-      'pass'      => $conn_pass,
-      'host'      => $conn_host,
-      'port'      => $conn_port,
-      'database'  => null,
-    );
-
-    $conn_raw = PhabricatorEnv::newObjectFromConfig(
-      'mysql.implementation',
-      array($config));
 
     try {
       queryfx($conn_raw, 'SELECT 1');
@@ -88,11 +77,8 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
         ->setIsFatal(true)
         ->addCommand(hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
     } else {
-
+      $conn_meta = $master->newApplicationConnection(
-      $config['database'] = $namespace.'_meta_data';
+        $namespace.'_meta_data');
-      $conn_meta = PhabricatorEnv::newObjectFromConfig(
-        'mysql.implementation',
-        array($config));
 
       $applied = queryfx_all($conn_meta, 'SELECT patch FROM patch_status');
       $applied = ipull($applied, 'patch', 'patch');
@@ -113,7 +99,6 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
       }
     }
 
-
     $host = PhabricatorEnv::getEnvConfig('mysql.host');
     $matches = null;
     if (preg_match('/^([^:]+):(\d+)$/', $host, $matches)) {

src/infrastructure/cluster/PhabricatorDatabaseRef.php

@@ -239,7 +239,7 @@ final class PhabricatorDatabaseRef
         continue;
       }
 
-      $conn = $ref->newConnection();
+      $conn = $ref->newManagementConnection();
 
       $t_start = microtime(true);
       try {
@@ -303,18 +303,69 @@ final class PhabricatorDatabaseRef
     return $refs;
   }
 
-  protected function newConnection() {
+  public function newManagementConnection() {
+    return $this->newConnection(
+      array(
+        'retries' => 0,
+        'timeout' => 3,
+      ));
+  }
+
+  public function newApplicationConnection($database) {
+    return $this->newConnection(
+      array(
+        'database' => $database,
+      ));
+  }
+
+  public static function getMasterDatabaseRef() {
+    $refs = self::loadAll();
+
+    if (!$refs) {
+      $conf = PhabricatorEnv::newObjectFromConfig(
+        'mysql.configuration-provider',
+        array(null, 'w', null));
+
+      return id(new self())
+        ->setHost($conf->getHost())
+        ->setPort($conf->getPort())
+        ->setUser($conf->getUser())
+        ->setPass($conf->getPassword())
+        ->setIsMaster(true);
+    }
+
+    $master = null;
+    foreach ($refs as $ref) {
+      if ($ref->getDisabled()) {
+        continue;
+      }
+      if ($ref->getIsMaster()) {
+        return $ref;
+      }
+    }
+
+    return null;
+  }
+
+  private function newConnection(array $options) {
+    $spec = $options + array(
+      'user' => $this->getUser(),
+      'pass' => $this->getPass(),
+      'host' => $this->getHost(),
+      'port' => $this->getPort(),
+      'database' => null,
+      'retries' => 3,
+      'timeout' => 15,
+    );
+
+    // TODO: Remove this once the MySQL connector has proper support
+    // for it, see T6710.
+    ini_set('mysql.connect_timeout', $spec['timeout']);
+
     return PhabricatorEnv::newObjectFromConfig(
       'mysql.implementation',
       array(
-        array(
+        $spec,
-          'user'      => $this->getUser(),
-          'pass'      => $this->getPass(),
-          'host'      => $this->getHost(),
-          'port'      => $this->getPort(),
-          'database'  => null,
-          'retries'   => 0,
-        ),
       ));
   }
 

src/infrastructure/storage/lisk/PhabricatorLiskDAO.php

@@ -52,32 +52,24 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
    */
   protected function establishLiveConnection($mode) {
     $namespace = self::getStorageNamespace();
-
+    $database = $namespace.'_'.$this->getApplicationName();
-    $conf = PhabricatorEnv::newObjectFromConfig(
-      'mysql.configuration-provider',
-      array($this, $mode, $namespace));
 
     $is_readonly = PhabricatorEnv::isReadOnly();
+
     if ($is_readonly && ($mode != 'r')) {
       throw new Exception(
         pht(
           'Attempting to establish write-mode connection from a read-only '.
           'page (to database "%s").',
-          $conf->getDatabase()));
+          $database));
     }
 
-    $connection = PhabricatorEnv::newObjectFromConfig(
+    $refs = PhabricatorDatabaseRef::loadAll();
-      'mysql.implementation',
+    if ($refs) {
-      array(
+      $connection = $this->newClusterConnection($database);
-        array(
+    } else {
-          'user'      => $conf->getUser(),
+      $connection = $this->newBasicConnection($database, $mode, $namespace);
-          'pass'      => $conf->getPassword(),
+    }
-          'host'      => $conf->getHost(),
-          'port'      => $conf->getPort(),
-          'database'  => $conf->getDatabase(),
-          'retries'   => 3,
-        ),
-      ));
 
     // TODO: This should be testing if the mode is "r", but that would proably
     // break a lot of things. Perform a more narrow test for readonly mode
@@ -90,6 +82,37 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
     return $connection;
   }
 
+  private function newBasicConnection($database, $mode, $namespace) {
+    $conf = PhabricatorEnv::newObjectFromConfig(
+      'mysql.configuration-provider',
+      array($this, $mode, $namespace));
+
+    return PhabricatorEnv::newObjectFromConfig(
+      'mysql.implementation',
+      array(
+        array(
+          'user'      => $conf->getUser(),
+          'pass'      => $conf->getPassword(),
+          'host'      => $conf->getHost(),
+          'port'      => $conf->getPort(),
+          'database'  => $database,
+          'retries'   => 3,
+        ),
+      ));
+  }
+
+  private function newClusterConnection($database) {
+    $master = PhabricatorDatabaseRef::getMasterDatabaseRef();
+
+    if (!$master) {
+      // TODO: Implicitly degrade to read-only mode.
+      throw new Exception(pht('No master in database cluster config!'));
+    }
+
+    return $master->newApplicationConnection($database);
+  }
+
+
   /**
    * @task config
    */