diff --git a/scripts/sql/manage_storage.php b/scripts/sql/manage_storage.php --- a/scripts/sql/manage_storage.php +++ b/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) { diff --git a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php --- a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php +++ b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php @@ -12,13 +12,52 @@ } 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( + 'phabricator/ $ ./bin/config set mysql.host %s', + $host)) + ->addCommand( + hsprintf( + 'phabricator/ $ ./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 @@ $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue( $database_exception); $this->addIssue($issue); - return; + return true; } $engines = queryfx_all($conn_raw, 'SHOW ENGINES'); @@ -54,7 +93,7 @@ ->setName(pht('MySQL InnoDB Engine Not Available')) ->setMessage($message) ->setIsFatal(true); - return; + return true; } $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace'); @@ -72,59 +111,29 @@ ->setMessage($message) ->setIsFatal(true) ->addCommand(hsprintf('phabricator/ $ ./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:
%s
", - phutil_implode_html(phutil_tag('br'), array_keys($diff)))) - ->addCommand( - hsprintf('phabricator/ $ ./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:
%s
", + phutil_implode_html(phutil_tag('br'), array_keys($diff)))) ->addCommand( - hsprintf( - 'phabricator/ $ ./bin/config set mysql.host %s', - $host)) - ->addCommand( - hsprintf( - 'phabricator/ $ ./bin/config set mysql.port %s', - $port)); + hsprintf('phabricator/ $ ./bin/storage upgrade')); + return true; } - } } diff --git a/src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php b/src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php --- a/src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php +++ b/src/infrastructure/cluster/PhabricatorClusterDatabasesConfigOptionType.php @@ -85,14 +85,6 @@ $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))); - } } } diff --git a/src/infrastructure/cluster/PhabricatorDatabaseRef.php b/src/infrastructure/cluster/PhabricatorDatabaseRef.php --- a/src/infrastructure/cluster/PhabricatorDatabaseRef.php +++ b/src/infrastructure/cluster/PhabricatorDatabaseRef.php @@ -446,24 +446,39 @@ 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 @@ ->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; @@ -499,10 +510,24 @@ 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) { diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -223,13 +223,24 @@ $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; + } } } diff --git a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php --- a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php +++ b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php @@ -114,7 +114,8 @@ } private function newClusterConnection($database, $mode) { - $master = PhabricatorDatabaseRef::getMasterDatabaseRef(); + $master = PhabricatorDatabaseRef::getMasterDatabaseRefForDatabase( + $database); if ($master && !$master->isSevered()) { $connection = $master->newApplicationConnection($database); @@ -130,7 +131,8 @@ } } - $replica = PhabricatorDatabaseRef::getReplicaDatabaseRef(); + $replica = PhabricatorDatabaseRef::getReplicaDatabaseRefForDatabase( + $database); if ($replica) { $connection = $replica->newApplicationConnection($database); $connection->setReadOnly(true);