From bc15eee3f2c536302b753e0e416f7cfda9908225 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 12 Nov 2016 09:22:10 -0800 Subject: [PATCH] Update SchemaQuery and the web UI to accommodate multiple master databases Summary: Depends on D16115. Ref T11044. In the brave new world of multiple masters, we need to check the schemata on each master when looking for missing storage patches, keys, schema changes, etc. This realigns all the "check out what's up with that schema" calls to work for multiple hosts, and updates the web UI to include a "Server" column and allow you to browse per-server. This doesn't update `bin/storage`, so it breaks things on its own (and unit tests probably won't pass). I'll update that in the next change. Test Plan: Configured local environment in cluster mode with multiple masters, saw both hosts' status reported in web UI. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11044 Differential Revision: https://secure.phabricator.com/D16847 --- .../PhabricatorConfigApplication.php | 3 +- .../PhabricatorConfigDatabaseController.php | 16 - ...abricatorConfigDatabaseIssueController.php | 88 ++--- ...bricatorConfigDatabaseStatusController.php | 307 ++++++++++++------ .../schema/PhabricatorConfigSchemaQuery.php | 91 ++++-- .../schema/PhabricatorConfigServerSchema.php | 10 + .../cluster/PhabricatorDatabaseRef.php | 11 + 7 files changed, 339 insertions(+), 187 deletions(-) diff --git a/src/applications/config/application/PhabricatorConfigApplication.php b/src/applications/config/application/PhabricatorConfigApplication.php index dda50a2b73..6b2704b0b4 100644 --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -45,9 +45,10 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { 'group/(?P[^/]+)/' => 'PhabricatorConfigGroupController', 'version/' => 'PhabricatorConfigVersionController', 'database/'. + '(?:(?P[^/]+)/'. '(?:(?P[^/]+)/'. '(?:(?P[^/]+)/'. - '(?:(?:col/(?P[^/]+)|key/(?P[^/]+))/)?)?)?' + '(?:(?:col/(?P[^/]+)|key/(?P[^/]+))/)?)?)?)?' => 'PhabricatorConfigDatabaseStatusController', 'dbissue/' => 'PhabricatorConfigDatabaseIssueController', '(?Pignore|unignore)/(?P[^/]+)/' diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseController.php b/src/applications/config/controller/PhabricatorConfigDatabaseController.php index c9f2aa6a3c..53af9a6b92 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseController.php @@ -3,22 +3,6 @@ abstract class PhabricatorConfigDatabaseController extends PhabricatorConfigController { - protected function buildSchemaQuery() { - $ref = PhabricatorDatabaseRef::getMasterDatabaseRef(); - - $api = id(new PhabricatorStorageManagementAPI()) - ->setUser($ref->getUser()) - ->setHost($ref->getHost()) - ->setPort($ref->getPort()) - ->setNamespace(PhabricatorLiskDAO::getDefaultStorageNamespace()) - ->setPassword($ref->getPass()); - - $query = id(new PhabricatorConfigSchemaQuery()) - ->setAPI($api); - - return $query; - } - protected function renderIcon($status) { switch ($status) { case PhabricatorConfigStorageSchema::STATUS_OKAY: diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php b/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php index f1a91d4d5b..7674d28f51 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php @@ -6,11 +6,11 @@ final class PhabricatorConfigDatabaseIssueController public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); - $query = $this->buildSchemaQuery(); + $query = new PhabricatorConfigSchemaQuery(); - $actual = $query->loadActualSchema(); - $expect = $query->loadExpectedSchema(); - $comp = $query->buildComparisonSchema($expect, $actual); + $actual = $query->loadActualSchemata(); + $expect = $query->loadExpectedSchemata(); + $comp_servers = $query->buildComparisonSchemata($expect, $actual); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Database Issues')); @@ -18,65 +18,70 @@ final class PhabricatorConfigDatabaseIssueController // Collect all open issues. $issues = array(); - foreach ($comp->getDatabases() as $database_name => $database) { - foreach ($database->getLocalIssues() as $issue) { - $issues[] = array( - $database_name, - null, - null, - null, - $issue, - ); - } - foreach ($database->getTables() as $table_name => $table) { - foreach ($table->getLocalIssues() as $issue) { + foreach ($comp_servers as $ref_name => $comp) { + foreach ($comp->getDatabases() as $database_name => $database) { + foreach ($database->getLocalIssues() as $issue) { $issues[] = array( + $ref_name, $database_name, - $table_name, + null, null, null, $issue, ); } - foreach ($table->getColumns() as $column_name => $column) { - foreach ($column->getLocalIssues() as $issue) { + foreach ($database->getTables() as $table_name => $table) { + foreach ($table->getLocalIssues() as $issue) { $issues[] = array( + $ref_name, $database_name, $table_name, - 'column', - $column_name, + null, + null, $issue, ); } - } - foreach ($table->getKeys() as $key_name => $key) { - foreach ($key->getLocalIssues() as $issue) { - $issues[] = array( - $database_name, - $table_name, - 'key', - $key_name, - $issue, - ); + foreach ($table->getColumns() as $column_name => $column) { + foreach ($column->getLocalIssues() as $issue) { + $issues[] = array( + $ref_name, + $database_name, + $table_name, + 'column', + $column_name, + $issue, + ); + } + } + foreach ($table->getKeys() as $key_name => $key) { + foreach ($key->getLocalIssues() as $issue) { + $issues[] = array( + $ref_name, + $database_name, + $table_name, + 'key', + $key_name, + $issue, + ); + } } } } } - // Sort all open issues so that the most severe issues appear first. $order = array(); $counts = array(); foreach ($issues as $key => $issue) { - $const = $issue[4]; + $const = $issue[5]; $status = PhabricatorConfigStorageSchema::getIssueStatus($const); $severity = PhabricatorConfigStorageSchema::getStatusSeverity($status); $order[$key] = sprintf( '~%d~%s%s%s', 9 - $severity, - $issue[0], $issue[1], - $issue[3]); + $issue[2], + $issue[4]); if (empty($counts[$status])) { $counts[$status] = 0; @@ -91,22 +96,25 @@ final class PhabricatorConfigDatabaseIssueController // Render the issues. $rows = array(); foreach ($issues as $issue) { - $const = $issue[4]; + $const = $issue[5]; + + $uri = $this->getApplicationURI('/database/'.$issue[0].'/'.$issue[1].'/'); $database_link = phutil_tag( 'a', array( - 'href' => $this->getApplicationURI('/database/'.$issue[0].'/'), + 'href' => $uri, ), - $issue[0]); + $issue[1]); $rows[] = array( $this->renderIcon( PhabricatorConfigStorageSchema::getIssueStatus($const)), + $issue[0], $database_link, - $issue[1], $issue[2], $issue[3], + $issue[4], PhabricatorConfigStorageSchema::getIssueDescription($const), ); } @@ -117,6 +125,7 @@ final class PhabricatorConfigDatabaseIssueController ->setHeaders( array( null, + pht('Server'), pht('Database'), pht('Table'), pht('Type'), @@ -130,6 +139,7 @@ final class PhabricatorConfigDatabaseIssueController null, null, null, + null, 'wide', )); diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php index bdeb254437..d49a546387 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php @@ -7,6 +7,7 @@ final class PhabricatorConfigDatabaseStatusController private $table; private $column; private $key; + private $ref; public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); @@ -14,48 +15,59 @@ final class PhabricatorConfigDatabaseStatusController $this->table = $request->getURIData('table'); $this->column = $request->getURIData('column'); $this->key = $request->getURIData('key'); + $this->ref = $request->getURIData('ref'); - $query = $this->buildSchemaQuery(); + $query = new PhabricatorConfigSchemaQuery(); - $actual = $query->loadActualSchema(); - $expect = $query->loadExpectedSchema(); - $comp = $query->buildComparisonSchema($expect, $actual); + $actual = $query->loadActualSchemata(); + $expect = $query->loadExpectedSchemata(); + $comp = $query->buildComparisonSchemata($expect, $actual); - if ($this->column) { - return $this->renderColumn( - $comp, - $expect, - $actual, - $this->database, - $this->table, - $this->column); - } else if ($this->key) { - return $this->renderKey( - $comp, - $expect, - $actual, - $this->database, - $this->table, - $this->key); - } else if ($this->table) { - return $this->renderTable( - $comp, - $expect, - $actual, - $this->database, - $this->table); - } else if ($this->database) { - return $this->renderDatabase( - $comp, - $expect, - $actual, - $this->database); - } else { - return $this->renderServer( - $comp, - $expect, - $actual); + if ($this->ref !== null) { + $server_actual = idx($actual, $this->ref); + if (!$server_actual) { + return new Aphront404Response(); + } + + $server_comparison = $comp[$this->ref]; + $server_expect = $expect[$this->ref]; + + if ($this->column) { + return $this->renderColumn( + $server_comparison, + $server_expect, + $server_actual, + $this->database, + $this->table, + $this->column); + } else if ($this->key) { + return $this->renderKey( + $server_comparison, + $server_expect, + $server_actual, + $this->database, + $this->table, + $this->key); + } else if ($this->table) { + return $this->renderTable( + $server_comparison, + $server_expect, + $server_actual, + $this->database, + $this->table); + } else if ($this->database) { + return $this->renderDatabase( + $server_comparison, + $server_expect, + $server_actual, + $this->database); + } } + + return $this->renderServers( + $comp, + $expect, + $actual); } private function buildResponse($title, $body) { @@ -66,34 +78,57 @@ final class PhabricatorConfigDatabaseStatusController $title = pht('Database Status'); } + $ref = $this->ref; + $database = $this->database; + $table = $this->table; + $column = $this->column; + $key = $this->key; + + $links = array(); + $links[] = array( + pht('Database Status'), + 'database/', + ); + + if ($database) { + $links[] = array( + $database, + "database/{$ref}/{$database}/", + ); + } + + if ($table) { + $links[] = array( + $table, + "database/{$ref}/{$database}/{$table}/", + ); + } + + if ($column) { + $links[] = array( + $column, + "database/{$ref}/{$database}/{$table}/col/{$column}/", + ); + } + + if ($key) { + $links[] = array( + $key, + "database/{$ref}/{$database}/{$table}/key/{$key}/", + ); + } + $crumbs = $this->buildApplicationCrumbs(); $crumbs->setBorder(true); - if ($this->database) { - $crumbs->addTextCrumb( - pht('Database Status'), - $this->getApplicationURI('database/')); - if ($this->table) { - $crumbs->addTextCrumb( - $this->database, - $this->getApplicationURI('database/'.$this->database.'/')); - if ($this->column || $this->key) { - $crumbs->addTextCrumb( - $this->table, - $this->getApplicationURI( - 'database/'.$this->database.'/'.$this->table.'/')); - if ($this->column) { - $crumbs->addTextCrumb($this->column); - } else { - $crumbs->addTextCrumb($this->key); - } - } else { - $crumbs->addTextCrumb($this->table); - } + + $last_key = last_key($links); + foreach ($links as $link_key => $link) { + list($name, $href) = $link; + if ($link_key == $last_key) { + $crumbs->addTextCrumb($name); } else { - $crumbs->addTextCrumb($this->database); + $crumbs->addTextCrumb($name, $this->getApplicationURI($href)); } - } else { - $crumbs->addTextCrumb(pht('Database Status')); } $doc_link = PhabricatorEnv::getDoclink('Managing Storage Adjustments'); @@ -121,52 +156,64 @@ final class PhabricatorConfigDatabaseStatusController } - private function renderServer( - PhabricatorConfigServerSchema $comp, - PhabricatorConfigServerSchema $expect, - PhabricatorConfigServerSchema $actual) { + private function renderServers( + array $comp_servers, + array $expect_servers, + array $actual_servers) { $charset_issue = PhabricatorConfigStorageSchema::ISSUE_CHARSET; $collation_issue = PhabricatorConfigStorageSchema::ISSUE_COLLATION; $rows = array(); - foreach ($comp->getDatabases() as $database_name => $database) { - $actual_database = $actual->getDatabase($database_name); - if ($actual_database) { - $charset = $actual_database->getCharacterSet(); - $collation = $actual_database->getCollation(); - } else { - $charset = null; - $collation = null; - } + foreach ($comp_servers as $ref_key => $comp) { + $actual = $actual_servers[$ref_key]; + $expect = $expect_servers[$ref_key]; + foreach ($comp->getDatabases() as $database_name => $database) { + $actual_database = $actual->getDatabase($database_name); + if ($actual_database) { + $charset = $actual_database->getCharacterSet(); + $collation = $actual_database->getCollation(); + } else { + $charset = null; + $collation = null; + } - $status = $database->getStatus(); - $issues = $database->getIssues(); + $status = $database->getStatus(); + $issues = $database->getIssues(); - $rows[] = array( - $this->renderIcon($status), - phutil_tag( - 'a', + $uri = $this->getURI( array( - 'href' => $this->getApplicationURI( - '/database/'.$database_name.'/'), - ), - $database_name), - $this->renderAttr($charset, $database->hasIssue($charset_issue)), - $this->renderAttr($collation, $database->hasIssue($collation_issue)), - ); + 'ref' => $ref_key, + 'database' => $database_name, + )); + + $rows[] = array( + $this->renderIcon($status), + $ref_key, + phutil_tag( + 'a', + array( + 'href' => $uri, + ), + $database_name), + $this->renderAttr($charset, $database->hasIssue($charset_issue)), + $this->renderAttr($collation, $database->hasIssue($collation_issue)), + ); + } } $table = id(new AphrontTableView($rows)) ->setHeaders( array( null, + pht('Server'), pht('Database'), pht('Charset'), pht('Collation'), )) ->setColumnClasses( array( + null, null, 'wide pri', null, @@ -200,13 +247,17 @@ final class PhabricatorConfigDatabaseStatusController foreach ($database->getTables() as $table_name => $table) { $status = $table->getStatus(); + $uri = $this->getURI( + array( + 'table' => $table_name, + )); + $rows[] = array( $this->renderIcon($status), phutil_tag( 'a', array( - 'href' => $this->getApplicationURI( - '/database/'.$database_name.'/'.$table_name.'/'), + 'href' => $uri, ), $table_name), $this->renderAttr( @@ -251,6 +302,10 @@ final class PhabricatorConfigDatabaseStatusController $properties = $this->buildProperties( array( + array( + pht('Server'), + $this->ref, + ), array( pht('Character Set'), $actual_charset, @@ -325,17 +380,17 @@ final class PhabricatorConfigDatabaseStatusController $data_type = $expect_column->getDataType(); } + $uri = $this->getURI( + array( + 'column' => $column_name, + )); + $rows[] = array( $this->renderIcon($status), phutil_tag( 'a', array( - 'href' => $this->getApplicationURI( - 'database/'. - $database_name.'/'. - $table_name.'/'. - 'col/'. - $column_name.'/'), + 'href' => $uri, ), $column_name), $data_type, @@ -407,17 +462,17 @@ final class PhabricatorConfigDatabaseStatusController $key->hasIssue($longkey_issue)); } + $uri = $this->getURI( + array( + 'key' => $key_name, + )); + $key_rows[] = array( $this->renderIcon($status), phutil_tag( 'a', array( - 'href' => $this->getApplicationURI( - 'database/'. - $database_name.'/'. - $table_name.'/'. - 'key/'. - $key_name.'/'), + 'href' => $uri, ), $key_name), $this->renderAttr( @@ -464,6 +519,10 @@ final class PhabricatorConfigDatabaseStatusController $properties = $this->buildProperties( array( + array( + pht('Server'), + $this->ref, + ), array( pht('Collation'), $actual_collation, @@ -561,6 +620,10 @@ final class PhabricatorConfigDatabaseStatusController $properties = $this->buildProperties( array( + array( + pht('Server'), + $this->ref, + ), array( pht('Data Type'), $data_type, @@ -678,6 +741,10 @@ final class PhabricatorConfigDatabaseStatusController $properties = $this->buildProperties( array( + array( + pht('Server'), + $this->ref, + ), array( pht('Unique'), $this->renderBoolean($actual_unique), @@ -745,4 +812,40 @@ final class PhabricatorConfigDatabaseStatusController return phutil_tag_div('config-page-property', $view); } + private function getURI(array $properties) { + $defaults = array( + 'ref' => $this->ref, + 'database' => $this->database, + 'table' => $this->table, + 'column' => $this->column, + 'key' => $this->key, + ); + + $properties = $properties + $defaults; + $properties = array_select_keys($properties, array_keys($defaults)); + + $parts = array(); + foreach ($properties as $key => $property) { + if (!strlen($property)) { + continue; + } + + if ($key == 'column') { + $parts[] = 'col'; + } else if ($key == 'key') { + $parts[] = 'key'; + } + + $parts[] = $property; + } + + if ($parts) { + $parts = implode('/', $parts).'/'; + } else { + $parts = null; + } + + return $this->getApplicationURI('/database/'.$parts); + } + } diff --git a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php index ca9b44cc07..0ef5e484b1 100644 --- a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php @@ -2,36 +2,40 @@ final class PhabricatorConfigSchemaQuery extends Phobject { - private $api; - - public function setAPI(PhabricatorStorageManagementAPI $api) { - $this->api = $api; - return $this; - } - - protected function getAPI() { - if (!$this->api) { - throw new PhutilInvalidStateException('setAPI'); - } - return $this->api; - } - - protected function getConn() { - return $this->getAPI()->getConn(null); - } - - private function getDatabaseNames() { - $api = $this->getAPI(); + private function getDatabaseNames(PhabricatorDatabaseRef $ref) { + $api = $this->getAPI($ref); $patches = PhabricatorSQLPatchList::buildAllPatches(); return $api->getDatabaseList( $patches, $only_living = true); } - public function loadActualSchema() { - $databases = $this->getDatabaseNames(); + private function getAPI(PhabricatorDatabaseRef $ref) { + return id(new PhabricatorStorageManagementAPI()) + ->setUser($ref->getUser()) + ->setHost($ref->getHost()) + ->setPort($ref->getPort()) + ->setNamespace(PhabricatorLiskDAO::getDefaultStorageNamespace()) + ->setPassword($ref->getPass()); + } + + public function loadActualSchemata() { + $refs = PhabricatorDatabaseRef::getMasterDatabaseRefs(); + + $schemata = array(); + foreach ($refs as $ref) { + $schema = $this->loadActualSchemaForServer($ref); + $schemata[$schema->getRef()->getRefKey()] = $schema; + } + + return $schemata; + } + + private function loadActualSchemaForServer(PhabricatorDatabaseRef $ref) { + $databases = $this->getDatabaseNames($ref); + + $conn = $ref->newManagementConnection(); - $conn = $this->getConn(); $tables = queryfx_all( $conn, 'SELECT TABLE_SCHEMA, TABLE_NAME, TABLE_COLLATION @@ -92,7 +96,8 @@ final class PhabricatorConfigSchemaQuery extends Phobject { // primary, unique, and foreign keys, so we can't use them here. We pull // indexes later on using SHOW INDEXES. - $server_schema = new PhabricatorConfigServerSchema(); + $server_schema = id(new PhabricatorConfigServerSchema()) + ->setRef($ref); $tables = igroup($tables, 'TABLE_SCHEMA'); foreach ($tables as $database_name => $database_tables) { @@ -177,15 +182,29 @@ final class PhabricatorConfigSchemaQuery extends Phobject { return $server_schema; } - public function loadExpectedSchema() { - $databases = $this->getDatabaseNames(); - $info = $this->getAPI()->getCharsetInfo(); + public function loadExpectedSchemata() { + $refs = PhabricatorDatabaseRef::getMasterDatabaseRefs(); + + $schemata = array(); + foreach ($refs as $ref) { + $schema = $this->loadExpectedSchemaForServer($ref); + $schemata[$schema->getRef()->getRefKey()] = $schema; + } + + return $schemata; + } + + public function loadExpectedSchemaForServer(PhabricatorDatabaseRef $ref) { + $databases = $this->getDatabaseNames($ref); + $info = $this->getAPI($ref)->getCharsetInfo(); $specs = id(new PhutilClassMapQuery()) ->setAncestorClass('PhabricatorConfigSchemaSpec') ->execute(); - $server_schema = new PhabricatorConfigServerSchema(); + $server_schema = id(new PhabricatorConfigServerSchema()) + ->setRef($ref); + foreach ($specs as $spec) { $spec ->setUTF8Charset( @@ -201,7 +220,21 @@ final class PhabricatorConfigSchemaQuery extends Phobject { return $server_schema; } - public function buildComparisonSchema( + public function buildComparisonSchemata( + array $expect_servers, + array $actual_servers) { + + $schemata = array(); + foreach ($actual_servers as $key => $actual_server) { + $schemata[$key] = $this->buildComparisonSchemaForServer( + $expect_servers[$key], + $actual_server); + } + + return $schemata; + } + + private function buildComparisonSchemaForServer( PhabricatorConfigServerSchema $expect, PhabricatorConfigServerSchema $actual) { diff --git a/src/applications/config/schema/PhabricatorConfigServerSchema.php b/src/applications/config/schema/PhabricatorConfigServerSchema.php index b8b21fe919..f067534b23 100644 --- a/src/applications/config/schema/PhabricatorConfigServerSchema.php +++ b/src/applications/config/schema/PhabricatorConfigServerSchema.php @@ -3,8 +3,18 @@ final class PhabricatorConfigServerSchema extends PhabricatorConfigStorageSchema { + private $ref; private $databases = array(); + public function setRef(PhabricatorDatabaseRef $ref) { + $this->ref = $ref; + return $this; + } + + public function getRef() { + return $this->ref; + } + public function addDatabase(PhabricatorConfigDatabaseSchema $database) { $key = $database->getName(); if (isset($this->databases[$key])) { diff --git a/src/infrastructure/cluster/PhabricatorDatabaseRef.php b/src/infrastructure/cluster/PhabricatorDatabaseRef.php index 2cab28104f..9033ba99ef 100644 --- a/src/infrastructure/cluster/PhabricatorDatabaseRef.php +++ b/src/infrastructure/cluster/PhabricatorDatabaseRef.php @@ -157,6 +157,17 @@ final class PhabricatorDatabaseRef return $this->isIndividual; } + public function getRefKey() { + $host = $this->getHost(); + + $port = $this->getPort(); + if (strlen($port)) { + return "{$host}:{$port}"; + } + + return $host; + } + public static function getConnectionStatusMap() { return array( self::STATUS_OKAY => array(