Repository: ambari Updated Branches: refs/heads/branch-2.5 2a5064d0a -> 04ec37d24
AMBARI-19957. Implement new DB checks for Postgres to prevent cross-schema confusion. (Balazs Bence Sari via stoader) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/04ec37d2 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/04ec37d2 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/04ec37d2 Branch: refs/heads/branch-2.5 Commit: 04ec37d24800a78d628920c8a53306f1bc54d5c0 Parents: 2a5064d Author: Balazs Bence Sari <bs...@hortonworks.com> Authored: Thu Feb 16 14:13:21 2017 +0100 Committer: Toader, Sebastian <stoa...@hortonworks.com> Committed: Thu Feb 16 14:13:40 2017 +0100 ---------------------------------------------------------------------- .../checks/DatabaseConsistencyCheckHelper.java | 299 ++++++++++--------- .../checks/DatabaseConsistencyCheckResult.java | 50 ++++ .../checks/DatabaseConsistencyChecker.java | 6 +- .../ambari/server/controller/AmbariServer.java | 43 +-- .../src/main/python/ambari_server_main.py | 23 +- .../DatabaseConsistencyCheckHelperTest.java | 148 ++++++++- 6 files changed, 384 insertions(+), 185 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java index 878d0fa..b4630a6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java @@ -20,7 +20,6 @@ package org.apache.ambari.server.checks; import java.io.File; import java.io.IOException; import java.sql.Connection; -import java.sql.DatabaseMetaData; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -31,9 +30,11 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Scanner; import java.util.Set; +import javax.annotation.Nullable; import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.Query; @@ -59,7 +60,9 @@ import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Splitter; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; import com.google.inject.Inject; import com.google.inject.Injector; @@ -77,21 +80,55 @@ public class DatabaseConsistencyCheckHelper { private static AmbariMetaInfo ambariMetaInfo; private static DBAccessor dbAccessor; + private static DatabaseConsistencyCheckResult checkResult = DatabaseConsistencyCheckResult.DB_CHECK_SUCCESS; - private static boolean errorsFound = false; - private static boolean warningsFound = false; - public static boolean ifErrorsFound() { - return errorsFound; + /** + * @return The result of the DB cheks run so far. + */ + public static DatabaseConsistencyCheckResult getLastCheckResult() { + return checkResult; + } + + /** + * Reset check result to {@link DatabaseConsistencyCheckResult#DB_CHECK_SUCCESS}. + */ + public static void resetCheckResult() { + checkResult = DatabaseConsistencyCheckResult.DB_CHECK_SUCCESS; } - public static boolean ifWarningsFound() { - return warningsFound; + /** + * Called internally to set the result of the DB checks. The new result is only recorded if it is more severe than + * the existing result. + * + * @param newResult the new result + */ + private static void setCheckResult(DatabaseConsistencyCheckResult newResult) { + if (newResult.ordinal() > checkResult.ordinal()) { + checkResult = newResult; + } } - public static void resetErrorWarningFlags() { - errorsFound = false; - warningsFound = false; + /** + * Called to indicate a warning during checks + * + * @param messageTemplate Message template (log4j format) + * @param messageParams Message params + */ + private static void warning(String messageTemplate, Object... messageParams) { + LOG.warn(messageTemplate, messageParams); + setCheckResult(DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + } + + /** + * Called to indicate an error during checks + * + * @param messageTemplate Message template (log4j format) + * @param messageParams Message params + */ + private static void error(String messageTemplate, Object... messageParams) { + LOG.error(messageTemplate, messageParams); + setCheckResult(DatabaseConsistencyCheckResult.DB_CHECK_ERROR); } public static void setInjector(Injector injector) { @@ -122,23 +159,29 @@ public class DatabaseConsistencyCheckHelper { } - public static void fixDatabaseConsistency() { - fixHostComponentStatesCountEqualsHostComponentsDesiredStates(); - fixClusterConfigsNotMappedToAnyService(); - } - - public static void runAllDBChecks() { + public static DatabaseConsistencyCheckResult runAllDBChecks(boolean fixIssues) throws Throwable { LOG.info("******************************* Check database started *******************************"); - checkSchemaName(); - checkMySQLEngine(); - checkForConfigsNotMappedToService(); - checkForNotMappedConfigsToCluster(); - checkForConfigsSelectedMoreThanOnce(); - checkForHostsWithoutState(); - checkHostComponentStatesCountEqualsHostComponentsDesiredStates(); - checkServiceConfigs(); - checkTopologyTables(); - LOG.info("******************************* Check database completed *******************************"); + try { + if (fixIssues) { + fixHostComponentStatesCountEqualsHostComponentsDesiredStates(); + fixClusterConfigsNotMappedToAnyService(); + } + checkSchemaName(); + checkMySQLEngine(); + checkForConfigsNotMappedToService(); + checkForNotMappedConfigsToCluster(); + checkForConfigsSelectedMoreThanOnce(); + checkForHostsWithoutState(); + checkHostComponentStatesCountEqualsHostComponentsDesiredStates(); + checkServiceConfigs(); + checkTopologyTables(); + LOG.info("******************************* Check database completed *******************************"); + return checkResult; + } + catch (Throwable ex) { + LOG.error("An error occurred during database consistency check.", ex); + throw ex; + } } public static void checkDBVersionCompatible() throws AmbariException { @@ -180,7 +223,7 @@ public class DatabaseConsistencyCheckHelper { LOG.info("DB store version is compatible"); } - public static void checkForNotMappedConfigsToCluster() { + static void checkForNotMappedConfigsToCluster() { LOG.info("Checking for configs not mapped to any cluster"); String GET_NOT_MAPPED_CONFIGS_QUERY = "select type_name from clusterconfig where type_name not in (select type_name from clusterconfigmapping)"; @@ -188,12 +231,7 @@ public class DatabaseConsistencyCheckHelper { ResultSet rs = null; Statement statement = null; - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + ensureConnection(); try { statement = connection.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE); @@ -204,8 +242,8 @@ public class DatabaseConsistencyCheckHelper { } } if (!nonSelectedConfigs.isEmpty()) { - LOG.warn("You have config(s): {} that is(are) not mapped (in clusterconfigmapping table) to any cluster!", StringUtils.join(nonSelectedConfigs, ",")); - warningsFound = true; + warning("You have config(s): {} that is(are) not mapped (in clusterconfigmapping table) to any cluster!", + nonSelectedConfigs); } } catch (SQLException e) { LOG.error("Exception occurred during check for not mapped configs to cluster procedure: ", e); @@ -234,7 +272,7 @@ public class DatabaseConsistencyCheckHelper { * it means that this version of config is actual. So, if any config type has more * than one selected version it's a bug and we are showing error message for user. * */ - public static void checkForConfigsSelectedMoreThanOnce() { + static void checkForConfigsSelectedMoreThanOnce() { LOG.info("Checking for configs selected more than once"); String GET_CONFIGS_SELECTED_MORE_THAN_ONCE_QUERY = "select c.cluster_name, ccm.type_name from clusterconfigmapping ccm " + @@ -245,12 +283,7 @@ public class DatabaseConsistencyCheckHelper { ResultSet rs = null; Statement statement = null; - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + ensureConnection(); try { statement = connection.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE); @@ -261,9 +294,8 @@ public class DatabaseConsistencyCheckHelper { } for (String clusterName : clusterConfigTypeMap.keySet()) { - LOG.error("You have config(s), in cluster {}, that is(are) selected more than once in clusterconfigmapping table: {}", + error("You have config(s), in cluster {}, that is(are) selected more than once in clusterconfigmapping table: {}", clusterName ,StringUtils.join(clusterConfigTypeMap.get(clusterName), ",")); - errorsFound = true; } } @@ -293,7 +325,7 @@ public class DatabaseConsistencyCheckHelper { * has related host state info in hoststate table. * If not then we are showing error. * */ - public static void checkForHostsWithoutState() { + static void checkForHostsWithoutState() { LOG.info("Checking for hosts without state"); String GET_HOSTS_WITHOUT_STATUS_QUERY = "select host_name from hosts where host_id not in (select host_id from hoststate)"; @@ -301,12 +333,7 @@ public class DatabaseConsistencyCheckHelper { ResultSet rs = null; Statement statement = null; - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + ensureConnection(); try { statement = connection.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE); @@ -317,8 +344,7 @@ public class DatabaseConsistencyCheckHelper { } if (!hostsWithoutStatus.isEmpty()) { - LOG.error("You have host(s) without state (in hoststate table): " + StringUtils.join(hostsWithoutStatus, ",")); - errorsFound = true; + error("You have host(s) without state (in hoststate table): " + StringUtils.join(hostsWithoutStatus, ",")); } } @@ -348,7 +374,7 @@ public class DatabaseConsistencyCheckHelper { * This method checks that for each row in topology_request there is at least one row in topology_logical_request, * topology_host_request, topology_host_task, topology_logical_task. * */ - public static void checkTopologyTables() { + static void checkTopologyTables() { LOG.info("Checking Topology tables"); String SELECT_REQUEST_COUNT_QUERY = "select count(tpr.id) from topology_request tpr"; @@ -389,10 +415,9 @@ public class DatabaseConsistencyCheckHelper { } if (topologyRequestCount != topologyRequestTablesJoinedCount) { - LOG.error("Your topology request hierarchy is not complete for each row in topology_request should exist " + + error("Your topology request hierarchy is not complete for each row in topology_request should exist " + "at least one raw in topology_logical_request, topology_host_request, topology_host_task, " + "topology_logical_task."); - errorsFound = true; } @@ -426,7 +451,7 @@ public class DatabaseConsistencyCheckHelper { * two tables should have the same count of rows. If not then we are * showing error for user. * */ - public static void checkHostComponentStatesCountEqualsHostComponentsDesiredStates() { + static void checkHostComponentStatesCountEqualsHostComponentsDesiredStates() { LOG.info("Checking host component states count equals host component desired states count"); String GET_HOST_COMPONENT_STATE_COUNT_QUERY = "select count(*) from hostcomponentstate"; @@ -439,12 +464,7 @@ public class DatabaseConsistencyCheckHelper { ResultSet rs = null; Statement statement = null; - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + ensureConnection(); try { statement = connection.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_UPDATABLE); @@ -471,10 +491,8 @@ public class DatabaseConsistencyCheckHelper { } if (hostComponentStateCount != hostComponentDesiredStateCount || hostComponentStateCount != mergedCount) { - LOG.error("Your host component states (hostcomponentstate table) count not equals host component desired states (hostcomponentdesiredstate table) count!"); - errorsFound = true; + error("Your host component states (hostcomponentstate table) count not equals host component desired states (hostcomponentdesiredstate table) count!"); } - } catch (SQLException e) { LOG.error("Exception occurred during check for same count of host component states and host component desired states: ", e); } finally { @@ -501,7 +519,7 @@ public class DatabaseConsistencyCheckHelper { * Remove configs that are not mapped to any service. */ @Transactional - public static void fixClusterConfigsNotMappedToAnyService() { + static void fixClusterConfigsNotMappedToAnyService() { LOG.info("Checking for configs not mapped to any Service"); ClusterDAO clusterDAO = injector.getInstance(ClusterDAO.class); List<ClusterConfigEntity> notMappedClusterConfigs = getNotMappedClusterConfigsToService(); @@ -534,7 +552,7 @@ public class DatabaseConsistencyCheckHelper { /** * Look for configs that are not mapped to any service. */ - public static void checkForConfigsNotMappedToService() { + static void checkForConfigsNotMappedToService() { LOG.info("Checking for configs that are not mapped to any service"); List<ClusterConfigEntity> notMappedClasterConfigs = getNotMappedClusterConfigsToService(); @@ -543,8 +561,7 @@ public class DatabaseConsistencyCheckHelper { nonMappedConfigs.add(clusterConfigEntity.getType() + '-' + clusterConfigEntity.getTag()); } if (!notMappedClasterConfigs.isEmpty()){ - LOG.warn("You have config(s): {} that is(are) not mapped (in serviceconfigmapping table) to any service!", StringUtils.join(nonMappedConfigs, ",")); - warningsFound = true; + warning("You have config(s): {} that is(are) not mapped (in serviceconfigmapping table) to any service!", StringUtils.join(nonMappedConfigs, ",")); } } @@ -555,7 +572,7 @@ public class DatabaseConsistencyCheckHelper { * adding missed host components. */ @Transactional - public static void fixHostComponentStatesCountEqualsHostComponentsDesiredStates() { + static void fixHostComponentStatesCountEqualsHostComponentsDesiredStates() { LOG.info("Checking that there are the same number of actual and desired host components"); HostComponentStateDAO hostComponentStateDAO = injector.getInstance(HostComponentStateDAO.class); @@ -617,69 +634,80 @@ public class DatabaseConsistencyCheckHelper { } /** - * This method checks db schema name for Postgres. - * */ - public static void checkSchemaName () { + * This makes the following checks for Postgres: + * <ol> + * <li>Check if the connection's schema (first item on search path) is the one set in ambari.properties</li> + * <li>Check if the connection's schema is present in the DB</li> + * <li>Check if the ambari tables exist in the schema configured in ambari.properties</li> + * <li>Check if ambari tables don't exist in other shemas</li> + * </ol> + * The purpose of these checks is to avoid that tables and constraints in ambari's schema get confused with tables + * and constraints in other schemas on the DB user's search path. This can happen after an improperly made DB restore + * operation and can cause issues during upgrade. + **/ + static void checkSchemaName () { Configuration conf = injector.getInstance(Configuration.class); - if(conf.getDatabaseType()!=Configuration.DatabaseType.POSTGRES) { - return; - } - LOG.info("Ensuring that the schema set for Postgres is correct"); - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); + if(conf.getDatabaseType() == Configuration.DatabaseType.POSTGRES) { + LOG.info("Ensuring that the schema set for Postgres is correct"); + + ensureConnection(); + + try (ResultSet schemaRs = connection.getMetaData().getSchemas(); + ResultSet searchPathRs = connection.createStatement().executeQuery("show search_path"); + ResultSet ambariTablesRs = connection.createStatement().executeQuery( + "select table_schema from information_schema.tables where table_name = 'hostcomponentdesiredstate'")) { + // Check if ambari's schema exists + final boolean ambariSchemaExists = getResultSetColumn(schemaRs, "TABLE_SCHEM").contains(conf.getDatabaseSchema()); + if ( !ambariSchemaExists ) { + warning("The schema [{}] defined for Ambari from ambari.properties has not been found in the database. " + + "Storing Ambari tables under a different schema can lead to problems.", conf.getDatabaseSchema()); + } + // Check if the right schema is first on the search path + List<Object> searchPathResultColumn = getResultSetColumn(searchPathRs, "search_path"); + List<String> searchPath = searchPathResultColumn.isEmpty() ? ImmutableList.<String>of() : + ImmutableList.copyOf(Splitter.on(",").trimResults().split(String.valueOf(searchPathResultColumn.get(0)))); + String firstSearchPathItem = searchPath.isEmpty() ? null : searchPath.get(0); + if (!Objects.equals(firstSearchPathItem, conf.getDatabaseSchema())) { + warning("The schema [{}] defined for Ambari in ambari.properties is not first on the search path:" + + " {}. This can lead to problems.", conf.getDatabaseSchema(), searchPath); + } + // Check schemas with Ambari tables. + ArrayList<Object> schemasWithAmbariTables = getResultSetColumn(ambariTablesRs, "table_schema"); + if ( ambariSchemaExists && !schemasWithAmbariTables.contains(conf.getDatabaseSchema()) ) { + warning("The schema [{}] defined for Ambari from ambari.properties does not contain the Ambari tables. " + + "Storing Ambari tables under a different schema can lead to problems.", conf.getDatabaseSchema()); + } + if ( schemasWithAmbariTables.size() > 1 ) { + warning("Multiple schemas contain the Ambari tables: {}. This can lead to problems.", schemasWithAmbariTables); + } + } + catch (SQLException e) { + warning("Exception occurred during checking db schema name: ", e); } - connection = dbAccessor.getConnection(); } - ResultSet rs = null; - try { - DatabaseMetaData databaseMetaData = connection.getMetaData(); - - rs = databaseMetaData.getSchemas(); + } - boolean ambariSchemaPresent = false; - if (rs != null) { - while (rs.next()) { - if(StringUtils.equals(rs.getString("TABLE_SCHEM"),conf.getDatabaseSchema())){ - ambariSchemaPresent = true; - break; - } - } - } - if (!ambariSchemaPresent){ - LOG.error("The schema %s defined for Ambari from ambari.properties has not been found in the database. " + - "This means that the Ambari tables are stored under the public schema which can lead to problems.", conf.getDatabaseSchema()); - warningsFound = true; - } - - } catch (SQLException e) { - LOG.error("Exception occurred during checking db schema name.: ", e); - } finally { - if (rs != null) { - try { - rs.close(); - } catch (SQLException e) { - LOG.error("Exception occurred during result set closing procedure: ", e); - } + private static ArrayList<Object> getResultSetColumn(@Nullable ResultSet rs, String columnName) throws SQLException { + ArrayList<Object> values = new ArrayList<>(); + if (null != rs) { + while (rs.next()) { + values.add(rs.getObject(columnName)); } } + return values; } /** * This method checks tables engine type to be innodb for MySQL. * */ - public static void checkMySQLEngine () { + static void checkMySQLEngine () { Configuration conf = injector.getInstance(Configuration.class); if(conf.getDatabaseType()!=Configuration.DatabaseType.MYSQL) { return; } LOG.info("Checking to ensure that the MySQL DB engine type is set to InnoDB"); - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + + ensureConnection(); String GET_INNODB_ENGINE_SUPPORT = "select TABLE_NAME, ENGINE from information_schema.tables where TABLE_SCHEMA = '%s' and LOWER(ENGINE) != 'innodb';"; @@ -692,11 +720,10 @@ public class DatabaseConsistencyCheckHelper { if (rs != null) { List<String> tablesInfo = new ArrayList<>(); while (rs.next()) { - errorsFound = true; tablesInfo.add(rs.getString("TABLE_NAME")); } if (!tablesInfo.isEmpty()){ - LOG.error("Found tables with engine type that is not InnoDB : %s", StringUtils.join(tablesInfo, ',')); + error("Found tables with engine type that is not InnoDB : {}", tablesInfo); } } } catch (SQLException e) { @@ -720,7 +747,7 @@ public class DatabaseConsistencyCheckHelper { * 4) Check if service has config which is not selected(has no actual config version) in clusterconfigmapping table. * If any issue was discovered, we are showing error message for user. * */ - public static void checkServiceConfigs() { + static void checkServiceConfigs() { LOG.info("Checking services and their configs"); String GET_SERVICES_WITHOUT_CONFIGS_QUERY = "select c.cluster_name, service_name from clusterservices cs " + @@ -754,12 +781,7 @@ public class DatabaseConsistencyCheckHelper { ResultSet rs = null; Statement statement = null; - if (connection == null) { - if (dbAccessor == null) { - dbAccessor = injector.getInstance(DBAccessor.class); - } - connection = dbAccessor.getConnection(); - } + ensureConnection(); LOG.info("Getting ambari metainfo instance"); if (ambariMetaInfo == null) { @@ -777,8 +799,7 @@ public class DatabaseConsistencyCheckHelper { } for (String clusterName : clusterServiceMap.keySet()) { - LOG.warn("Service(s): {}, from cluster {} has no config(s) in serviceconfig table!", StringUtils.join(clusterServiceMap.get(clusterName), ","), clusterName); - warningsFound = true; + warning("Service(s): {}, from cluster {} has no config(s) in serviceconfig table!", StringUtils.join(clusterServiceMap.get(clusterName), ","), clusterName); } } @@ -804,8 +825,7 @@ public class DatabaseConsistencyCheckHelper { for (String clName : clusterServiceVersionMap.keySet()) { Multimap<String, String> serviceVersion = clusterServiceVersionMap.get(clName); for (String servName : serviceVersion.keySet()) { - LOG.error("In cluster {}, service config mapping is unavailable (in table serviceconfigmapping) for service {} with version(s) {}! ", clName, servName, StringUtils.join(serviceVersion.get(servName), ",")); - errorsFound = true; + error("In cluster {}, service config mapping is unavailable (in table serviceconfigmapping) for service {} with version(s) {}! ", clName, servName, StringUtils.join(serviceVersion.get(servName), ",")); } } @@ -880,9 +900,8 @@ public class DatabaseConsistencyCheckHelper { stackServiceConfigs.put(serviceName, configType); } } else { - LOG.warn("Service {} is not available for stack {} in cluster {}", + warning("Service {} is not available for stack {} in cluster {}", serviceName, stackName + "-" + stackVersion, clusterName); - warningsFound = true; } } @@ -899,9 +918,8 @@ public class DatabaseConsistencyCheckHelper { if (serviceConfigsFromDB != null && serviceConfigsFromStack != null) { serviceConfigsFromStack.removeAll(serviceConfigsFromDB); if (!serviceConfigsFromStack.isEmpty()) { - LOG.error("Required config(s): {} is(are) not available for service {} with service config version {} in cluster {}", + error("Required config(s): {} is(are) not available for service {} with service config version {} in cluster {}", StringUtils.join(serviceConfigsFromStack, ","), serviceName, Integer.toString(serviceVersion), clusterName); - errorsFound = true; } } } @@ -938,8 +956,7 @@ public class DatabaseConsistencyCheckHelper { for (String clusterName : clusterServiceConfigType.keySet()) { Multimap<String, String> serviceConfig = clusterServiceConfigType.get(clusterName); for (String serviceName : serviceConfig.keySet()) { - LOG.error("You have non selected configs: {} for service {} from cluster {}!", StringUtils.join(serviceConfig.get(serviceName), ","), serviceName, clusterName); - errorsFound = true; + error("You have non selected configs: {} for service {} from cluster {}!", StringUtils.join(serviceConfig.get(serviceName), ","), serviceName, clusterName); } } } catch (SQLException e) { @@ -966,5 +983,13 @@ public class DatabaseConsistencyCheckHelper { } + private static void ensureConnection() { + if (connection == null) { + if (dbAccessor == null) { + dbAccessor = injector.getInstance(DBAccessor.class); + } + connection = dbAccessor.getConnection(); + } + } } http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckResult.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckResult.java b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckResult.java new file mode 100644 index 0000000..7291d5d --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckResult.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ambari.server.checks; + +/** + * Enum representing the possible outcomes of the on-start database consistency check. + * + * <p><b>IMPORTANT:</b></p> + * <ul> + * <li>Outcomes are ordered by severity, the program relies on this.</li> + * <li>The check result is logged to the standard output and the server startup python script relies + * on them. When changing the values, please make sure the startup scripts are changed accordingly!</li> + * </ul> + + */ +public enum DatabaseConsistencyCheckResult { + DB_CHECK_SUCCESS, + DB_CHECK_WARNING, + DB_CHECK_ERROR; + + /** + * @return a boolean indicating that the result is has least warning severity + */ + public boolean isErrorOrWarning() { + return this == DB_CHECK_WARNING || this == DB_CHECK_ERROR; + } + + /** + * @return a boolean indicating that the result is error + */ + public boolean isError() { + return this == DB_CHECK_ERROR; + } + +} http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java index f12dd50..2d750f4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java @@ -103,7 +103,7 @@ public class DatabaseConsistencyChecker { databaseConsistencyChecker.startPersistenceService(); - DatabaseConsistencyCheckHelper.runAllDBChecks(); + DatabaseConsistencyCheckHelper.runAllDBChecks(false); databaseConsistencyChecker.stopPersistenceService(); @@ -117,7 +117,7 @@ public class DatabaseConsistencyChecker { } } finally { DatabaseConsistencyCheckHelper.closeConnection(); - if (DatabaseConsistencyCheckHelper.ifErrorsFound() || DatabaseConsistencyCheckHelper.ifWarningsFound()) { + if (DatabaseConsistencyCheckHelper.getLastCheckResult().isErrorOrWarning()) { String ambariDBConsistencyCheckLog = "ambari-server-check-database.log"; if (LOG instanceof Log4jLoggerAdapter) { org.apache.log4j.Logger dbConsistencyCheckHelperLogger = org.apache.log4j.Logger.getLogger(DatabaseConsistencyCheckHelper.class); @@ -132,7 +132,7 @@ public class DatabaseConsistencyChecker { } ambariDBConsistencyCheckLog = ambariDBConsistencyCheckLog.replace("//", "/"); - if (DatabaseConsistencyCheckHelper.ifErrorsFound()) { + if (DatabaseConsistencyCheckHelper.getLastCheckResult().isError()) { System.out.print(String.format("DB configs consistency check failed. Run \"ambari-server start --skip-database-check\" to skip. " + "You may try --auto-fix-database flag to attempt to fix issues automatically. " + "If you use this \"--skip-database-check\" option, do not make any changes to your cluster topology " + http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java index 535940c..b5acdce 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java @@ -57,6 +57,7 @@ import org.apache.ambari.server.audit.AuditLoggerModule; import org.apache.ambari.server.audit.request.RequestAuditLogger; import org.apache.ambari.server.bootstrap.BootStrapImpl; import org.apache.ambari.server.checks.DatabaseConsistencyCheckHelper; +import org.apache.ambari.server.checks.DatabaseConsistencyCheckResult; import org.apache.ambari.server.configuration.ComponentSSLConfiguration; import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.controller.internal.AbstractControllerResourceProvider; @@ -670,36 +671,20 @@ public class AmbariServer { */ protected void runDatabaseConsistencyCheck() throws Exception { if (System.getProperty("skipDatabaseConsistencyCheck") == null) { - System.out.println("Database consistency check started"); - Logger DB_CHECK_LOG = LoggerFactory.getLogger(DatabaseConsistencyCheckHelper.class); - try{ - if (System.getProperty("fixDatabaseConsistency") != null ){ - DatabaseConsistencyCheckHelper.fixDatabaseConsistency(); - } - DatabaseConsistencyCheckHelper.runAllDBChecks(); - } catch(Throwable e) { - System.out.println("Database consistency check: failed"); - if (e instanceof AmbariException) { - DB_CHECK_LOG.error("Exception occurred during database check:", e); - System.out.println("Exception occurred during database check: " + e.getMessage()); - e.printStackTrace(); - throw (AmbariException)e; - } else { - DB_CHECK_LOG.error("Unexpected error, database check failed", e); - System.out.println("Unexpected error, database check failed: " + e.getMessage()); - e.printStackTrace(); - throw new Exception("Unexpected error, database check failed", e); - } - } finally { - if (DatabaseConsistencyCheckHelper.ifErrorsFound()) { - System.out.println("Database consistency check: failed"); + boolean fixIssues = (System.getProperty("fixDatabaseConsistency") != null); + try { + DatabaseConsistencyCheckResult checkResult = DatabaseConsistencyCheckHelper.runAllDBChecks(fixIssues); + // Writing explicitly to the console is necessary as the python start script expects it. + System.out.println("Database consistency check result: " + checkResult); + if (checkResult.isError()) { System.exit(1); - } else if (DatabaseConsistencyCheckHelper.ifWarningsFound()) { - System.out.println("Database consistency check: warning"); - } else { - System.out.println("Database consistency check: successful"); } } + catch (Throwable ex) { + // Writing explicitly to the console is necessary as the python start script expects it. + System.out.println("Database consistency check result: " + DatabaseConsistencyCheckResult.DB_CHECK_ERROR); + throw new Exception(ex); + } } } @@ -1007,7 +992,9 @@ public class AmbariServer { setupProxyAuth(); injector.getInstance(GuiceJpaInitializer.class); + DatabaseConsistencyCheckHelper.checkDBVersionCompatible(); + server = injector.getInstance(AmbariServer.class); injector.getInstance(UpdateActiveRepoVersionOnStartup.class).process(); CertificateManager certMan = injector.getInstance(CertificateManager.class); @@ -1024,4 +1011,6 @@ public class AmbariServer { System.exit(-1); } } + + } http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/main/python/ambari_server_main.py ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/python/ambari_server_main.py b/ambari-server/src/main/python/ambari_server_main.py index c7ae48a..66678b0 100644 --- a/ambari-server/src/main/python/ambari_server_main.py +++ b/ambari-server/src/main/python/ambari_server_main.py @@ -239,16 +239,19 @@ def wait_for_server_start(pidFile, scmStatus): exception = FatalException(-1, AMBARI_SERVER_NOT_STARTED_MSG) if os.path.isfile(configDefaults.SERVER_OUT_FILE): - if 'Database consistency check: failed' in open(configDefaults.SERVER_OUT_FILE).read(): - print "DB configs consistency check failed. Run \"ambari-server start --skip-database-check\" to skip. " \ - "You may try --auto-fix-database flag to attempt to fix issues automatically. " \ - "If you use this \"--skip-database-check\" option, do not make any changes to your cluster topology " \ - "or perform a cluster upgrade until you correct the database consistency issues. See " + \ - configDefaults.DB_CHECK_LOG + " for more details on the consistency issues." - elif 'Database consistency check: warning' in open(configDefaults.SERVER_OUT_FILE).read(): - print "DB configs consistency check found warnings. See " + configDefaults.DB_CHECK_LOG + " for more details." - else: - print "DB configs consistency check: no errors and warnings were found." + if 'DB_CHECK_ERROR' in open(configDefaults.SERVER_OUT_FILE).read(): + print "\nDB configs consistency check failed. Run \"ambari-server start --skip-database-check\" to skip. " \ + "You may try --auto-fix-database flag to attempt to fix issues automatically. " \ + "If you use this \"--skip-database-check\" option, do not make any changes to your cluster topology " \ + "or perform a cluster upgrade until you correct the database consistency issues. See " + \ + configDefaults.DB_CHECK_LOG + " for more details on the consistency issues." + elif 'DB_CHECK_WARNING' in open(configDefaults.SERVER_OUT_FILE).read(): + print "\nDB configs consistency check found warnings. See " + configDefaults.DB_CHECK_LOG + " for more details." + # Only presume that DB check was successful if it explicitly appears in the log. An unexpected error may prevent + # the consistency check from running at all, so missing error/warning message in the log cannot imply the check was + # successful + elif 'DB_CHECK_SUCCESS' in open(configDefaults.SERVER_OUT_FILE).read(): + print "\nDB configs consistency check: no errors and warnings were found." else: sys.stdout.write(configDefaults.SERVER_OUT_FILE + " does not exist") http://git-wip-us.apache.org/repos/asf/ambari/blob/04ec37d2/ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java index 40ebb7f..455ce01 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java @@ -18,29 +18,44 @@ package org.apache.ambari.server.checks; -import javax.persistence.EntityManager; -import junit.framework.Assert; +import static com.google.common.collect.Lists.newArrayList; +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.createStrictMock; + import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.sql.Connection; +import java.sql.DatabaseMetaData; import java.sql.ResultSet; +import java.sql.SQLException; import java.sql.Statement; import java.util.HashMap; +import java.util.List; import java.util.Map; +import javax.persistence.EntityManager; + import org.apache.ambari.server.api.services.AmbariMetaInfo; +import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.orm.DBAccessor; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.ServiceInfo; import org.apache.ambari.server.state.stack.OsFamily; import org.easymock.EasyMockSupport; +import org.junit.Assert; import org.junit.Test; +import com.google.common.collect.Lists; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; + public class DatabaseConsistencyCheckHelperTest { @Test @@ -172,13 +187,13 @@ public class DatabaseConsistencyCheckHelperTest { @Test public void testCheckTopologyTablesAreConsistent() throws Exception { testCheckTopologyTablesConsistent(2); - Assert.assertTrue(!DatabaseConsistencyCheckHelper.ifErrorsFound()); + Assert.assertFalse(DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); } @Test public void testCheckTopologyTablesAreNotConsistent() throws Exception { testCheckTopologyTablesConsistent(1); - Assert.assertTrue(DatabaseConsistencyCheckHelper.ifErrorsFound()); + Assert.assertTrue(DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); } private void testCheckTopologyTablesConsistent(int resultCount) throws Exception { @@ -353,6 +368,123 @@ public class DatabaseConsistencyCheckHelperTest { } @Test + public void testSchemaName_NoIssues() throws Exception { + setupMocksForTestSchemaName("ambari", "ambari, public", newArrayList("ambari", "public"), newArrayList("ambari")); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertFalse("No warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + @Test + public void testSchemaName_WrongSearchPathOrder() throws Exception { + setupMocksForTestSchemaName("ambari", "public, ambari", newArrayList("ambari", "public"), newArrayList("ambari")); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + @Test + public void testSchemaName_NoSearchPath() throws Exception { + setupMocksForTestSchemaName("ambari", null, newArrayList("ambari", "public"), newArrayList("ambari")); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + + @Test + public void testSchemaName_NoAmbariSchema() throws Exception { + setupMocksForTestSchemaName("ambari", null, newArrayList("public"), Lists.<String>newArrayList()); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + @Test + public void testSchemaName_NoTablesInAmbariSchema() throws Exception { + setupMocksForTestSchemaName("ambari", "ambari", newArrayList("ambari", "public"), newArrayList("public")); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + @Test + public void testSchemaName_AmbariTablesInMultipleSchemas() throws Exception { + setupMocksForTestSchemaName("ambari", "ambari", newArrayList("ambari", "public"), newArrayList("ambari", "public")); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + @Test + public void testSchemaName_NullsAreTolerated() throws Exception { + setupMocksForTestSchemaName(null, null, null, null); + DatabaseConsistencyCheckHelper.checkSchemaName(); + assertTrue("Warnings were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult() == + DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + assertFalse("No errors were expected.", DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); + } + + private void setupMocksForTestSchemaName(String configuredSchema, String searchPath, List<String> schemas, + List<String> schemasWithAmbariTables) throws Exception { + final Configuration config = createNiceMock(Configuration.class); + final OsFamily osFamily = createNiceMock(OsFamily.class); + final Connection connection = createNiceMock(Connection.class); + final DBAccessor dbAccessor = createStrictMock(DBAccessor.class); + final Statement searchPathStatement = createStrictMock(Statement.class); + final Statement getTablesStatement = createStrictMock(Statement.class); + final DatabaseMetaData dbMetaData = createStrictMock(DatabaseMetaData.class); + final Injector mockInjector = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + bind(DBAccessor.class).toInstance(dbAccessor); + bind(OsFamily.class).toInstance(osFamily); + bind(Configuration.class).toInstance(config); + } + }); + expect(config.getDatabaseSchema()).andReturn(configuredSchema).anyTimes(); + expect(config.getDatabaseType()).andReturn(Configuration.DatabaseType.POSTGRES); + expect(dbAccessor.getConnection()).andReturn(connection); + expect(connection.getMetaData()).andReturn(dbMetaData); + expect(connection.createStatement()).andReturn(searchPathStatement); + expect(connection.createStatement()).andReturn(getTablesStatement); + expect(dbMetaData.getSchemas()).andReturn(resultSet("TABLE_SCHEM", schemas)); + expect(searchPathStatement.executeQuery(anyString())).andReturn( + resultSet("search_path", newArrayList(searchPath))); + expect(getTablesStatement.executeQuery(anyString())).andReturn( + resultSet("table_schema", schemasWithAmbariTables)); + replay(config, connection, dbAccessor, dbMetaData, getTablesStatement, osFamily, searchPathStatement); + DatabaseConsistencyCheckHelper.setInjector(mockInjector); + DatabaseConsistencyCheckHelper.setConnection(null); + DatabaseConsistencyCheckHelper.resetCheckResult(); + } + + private ResultSet resultSet(final String columnName, final List<? extends Object> columnData) throws SQLException { + if (null == columnData) { + return null; + } + else { + ResultSet rs = createNiceMock(ResultSet.class); + if ( !columnData.isEmpty() ) { + expect(rs.next()).andReturn(true).times(columnData.size()); + } + expect(rs.next()).andReturn(false); + for(Object item: columnData) { + expect(rs.getObject(columnName)).andReturn(item); + } + replay(rs); + return rs; + } + } + + + @Test public void testCheckServiceConfigs_missingServiceConfigGeneratesWarning() throws Exception { EasyMockSupport easyMockSupport = new EasyMockSupport(); final AmbariMetaInfo mockAmbariMetainfo = easyMockSupport.createNiceMock(AmbariMetaInfo.class); @@ -431,16 +563,16 @@ public class DatabaseConsistencyCheckHelperTest { mockAmbariMetainfo.init(); - DatabaseConsistencyCheckHelper.resetErrorWarningFlags(); + DatabaseConsistencyCheckHelper.resetCheckResult(); DatabaseConsistencyCheckHelper.checkServiceConfigs(); easyMockSupport.verifyAll(); Assert.assertTrue("Missing service config for OPENSOFT R should have triggered a warning.", - DatabaseConsistencyCheckHelper.ifWarningsFound()); - Assert.assertFalse("No errors should have been triggered.", DatabaseConsistencyCheckHelper.ifErrorsFound()); + DatabaseConsistencyCheckHelper.getLastCheckResult() == DatabaseConsistencyCheckResult.DB_CHECK_WARNING); + Assert.assertFalse("No errors should have been triggered.", + DatabaseConsistencyCheckHelper.getLastCheckResult().isError()); } } -
