This is an automated email from the ASF dual-hosted git repository. gjacoby pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push: new 45929b5 PHOENIX-5922 - IndexUpgradeTool should always re-enable tables on failure 45929b5 is described below commit 45929b5a74feddb647571c7d7c057142973b45b0 Author: Geoffrey Jacoby <gjac...@apache.org> AuthorDate: Wed May 27 16:13:12 2020 -0700 PHOENIX-5922 - IndexUpgradeTool should always re-enable tables on failure --- .../end2end/ParameterizedIndexUpgradeToolIT.java | 46 +++++++++ .../phoenix/mapreduce/index/IndexUpgradeTool.java | 107 +++++++++++++++------ 2 files changed, 122 insertions(+), 31 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java index 5479576..2ed249e 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java @@ -381,6 +381,52 @@ public class ParameterizedIndexUpgradeToolIT extends BaseTest { } } + @Test + public void testRollbackAfterFailure() throws Exception { + validate(true); + if (upgrade) { + iut.setFailUpgradeTask(true); + } else { + iut.setFailDowngradeTask(true); + } + iut.prepareToolSetup(); + int status = iut.executeTool(); + Assert.assertEquals(-1, status); + //should have rolled back and be in the same status we started with + validate(true); + } + + @Test + public void testTableReenableAfterDoubleFailure() throws Exception { + validate(true); + //this will force the upgrade/downgrade to fail, and then the rollback to fail too + //we want to make sure that even then, we'll try to re-enable the HBase tables + iut.setFailUpgradeTask(true); + iut.setFailDowngradeTask(true); + iut.prepareToolSetup(); + try { + iut.executeTool(); + } catch (RuntimeException e) { + //double failures throw an exception so that the tool stops immediately + validateTablesEnabled(INPUT_LIST); + return; + } + Assert.fail("Should have thrown an exception!"); + } + + private void validateTablesEnabled(String inputList) throws IOException, SQLException { + Admin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin(); + String[] tableNames = inputList.split(","); + Assert.assertNotNull(tableNames); + Assert.assertTrue(tableNames.length > 0); + for (String tableName : tableNames) { + String physicalTableName = + SchemaUtil.getPhysicalHBaseTableName(SchemaUtil.getSchemaNameFromFullName(tableName), + SchemaUtil.getTableNameFromFullName(tableName), isNamespaceEnabled).getString(); + Assert.assertTrue(admin.isTableEnabled(TableName.valueOf(physicalTableName))); + } + } + @After public void cleanup() throws IOException, SQLException { if (conn == null) { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java index a856b5e..fca7930 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java @@ -26,6 +26,8 @@ import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; import org.apache.commons.cli.ParseException; +import org.apache.commons.cli.PosixParser; +import org.apache.commons.lang.StringUtils; import org.apache.hadoop.conf.Configured; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.client.CoprocessorDescriptorBuilder; @@ -60,6 +62,8 @@ import java.sql.ResultSet; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.logging.Logger; import org.apache.hadoop.conf.Configuration; import org.apache.phoenix.util.SchemaUtil; @@ -131,6 +135,10 @@ public class IndexUpgradeTool extends Configured implements Tool { private String indexToolOpts; private boolean test = false; + private boolean isWaitComplete = false; + private boolean failUpgradeTask = false; + private boolean failDowngradeTask = false; + private boolean hasFailure = false; public void setDryRun(boolean dryRun) { this.dryRun = dryRun; @@ -170,6 +178,15 @@ public class IndexUpgradeTool extends Configured implements Tool { public String getIndexToolOpts() { return this.indexToolOpts; } + @VisibleForTesting + public void setFailUpgradeTask(boolean failInitialTask) { + this.failUpgradeTask = failInitialTask; + } + + public void setFailDowngradeTask(boolean failRollbackTask) { + this.failDowngradeTask = failRollbackTask; + } + public IndexUpgradeTool(String mode, String tables, String inputFile, String outputFile, boolean dryRun, IndexTool indexTool, boolean rebuild) { this.operation = mode; @@ -195,7 +212,12 @@ public class IndexUpgradeTool extends Configured implements Tool { initializeTool(cmdLine); prepareToolSetup(); executeTool(); - return 0; + if (hasFailure) { + return -1; + } else { + return 0; + } + } /** @@ -209,7 +231,7 @@ public class IndexUpgradeTool extends Configured implements Tool { final Options options = getOptions(); - CommandLineParser parser = new DefaultParser(); + CommandLineParser parser = new PosixParser(); CommandLine cmdLine = null; try { cmdLine = parser.parse(options, args); @@ -321,7 +343,7 @@ public class IndexUpgradeTool extends Configured implements Tool { LOGGER.info("This is the beginning of the tool with dry run."); } } catch (IOException e) { - LOGGER.severe("Something went wrong "+e); + LOGGER.severe("Something went wrong " + e); System.exit(-1); } } @@ -398,15 +420,20 @@ public class IndexUpgradeTool extends Configured implements Tool { executeToolForMutableTables(conn, queryServices, conf, mutableList); enableImmutableTables(queryServices, immutableList, startWaitTime); rebuildIndexes(conn, conf, immutableList); - return 0; + if (hasFailure) { + return -1; + } else { + return 0; + } } private long executeToolForImmutableTables(ConnectionQueryServices queryServices, - ArrayList<String> immutableList) { + List<String> immutableList) { if (immutableList.isEmpty()) { return 0; } LOGGER.info("Started " + operation + " for immutable tables"); + List<String> failedTables = new ArrayList<String>(); for (String dataTableFullName : immutableList) { try (Admin admin = queryServices.getAdmin()) { HashSet<String> indexes = tablesAndIndexes.get(dataTableFullName); @@ -414,12 +441,13 @@ public class IndexUpgradeTool extends Configured implements Tool { + " (immutable)"); disableTable(admin, dataTableFullName, indexes); modifyTable(admin, dataTableFullName, indexes); - } catch (IOException | SQLException e) { + } catch (Throwable e) { LOGGER.severe("Something went wrong while disabling " + "or modifying immutable table " + e); - handleFailure(queryServices, dataTableFullName, immutableList); + handleFailure(queryServices, dataTableFullName, immutableList, failedTables); } } + immutableList.removeAll(failedTables); long startWaitTime = EnvironmentEdgeManager.currentTimeMillis(); return startWaitTime; } @@ -432,6 +460,7 @@ public class IndexUpgradeTool extends Configured implements Tool { return; } LOGGER.info("Started " + operation + " for mutable tables"); + List<String> failedTables = new ArrayList<>(); for (String dataTableFullName : mutableTables) { try (Admin admin = queryServices.getAdmin()) { HashSet<String> indexes = tablesAndIndexes.get(dataTableFullName); @@ -440,19 +469,22 @@ public class IndexUpgradeTool extends Configured implements Tool { modifyTable(admin, dataTableFullName, indexes); enableTable(admin, dataTableFullName, indexes); LOGGER.info("Completed " + operation + " of " + dataTableFullName); - } catch (IOException | SQLException e) { + } catch (Throwable e) { LOGGER.severe("Something went wrong while executing " - + operation + " steps for "+ dataTableFullName + " " + e); - handleFailure(queryServices, dataTableFullName, mutableTables); + + operation + " steps for "+ dataTableFullName + " " + e); + handleFailure(queryServices, dataTableFullName, mutableTables, failedTables); } } + mutableTables.removeAll(failedTables); // Opportunistically kick-off index rebuilds after upgrade operation rebuildIndexes(conn, conf, mutableTables); } private void handleFailure(ConnectionQueryServices queryServices, String dataTableFullName, - ArrayList<String> tableList) { + List<String> tableList, + List<String> failedTables) { + hasFailure = true; LOGGER.info("Performing error handling to revert the steps taken during " + operation); HashSet<String> indexes = tablesAndIndexes.get(dataTableFullName); try (Admin admin = queryServices.getAdmin()) { @@ -463,14 +495,21 @@ public class IndexUpgradeTool extends Configured implements Tool { upgrade = !upgrade; tablesAndIndexes.remove(dataTableFullName); //removing from the map - tableList.remove(dataTableFullName); //removing from the list + failedTables.add(dataTableFullName); //everything in failed tables will later be + // removed from the list LOGGER.severe(dataTableFullName+" has been removed from the list as tool failed" + " to perform "+operation); - } catch (IOException | SQLException e) { + } catch (Throwable e) { LOGGER.severe("Revert of the "+operation +" failed in error handling, " - + "throwing runtime exception"); + + "re-enabling tables and then throwing runtime exception"); LOGGER.severe("Confirm the state for "+getSubListString(tableList, dataTableFullName)); + try { + enableTable(queryServices.getAdmin(), dataTableFullName, indexes); + } catch (Exception ex) { + throw new RuntimeException("Error re-enabling tables after rollback failure. " + + "Original exception that caused the rollback: [" + e.toString() + " " + "]", ex); + } throw new RuntimeException(e); } } @@ -514,9 +553,9 @@ public class IndexUpgradeTool extends Configured implements Tool { } } - private String getSubListString(ArrayList<String> tableList, String dataTableFullName) { - return StringUtils.join(",", tableList.subList(tableList.indexOf(dataTableFullName), - tableList.size())); + private String getSubListString(List<String> tableList, String dataTableFullName) { + return StringUtils.join(tableList.subList(tableList.indexOf(dataTableFullName), + tableList.size()), ","); } private long getWaitMoreTime(long startWaitTime) { @@ -528,17 +567,6 @@ public class IndexUpgradeTool extends Configured implements Tool { return (((waitTime) * 60000) - Math.abs(endWaitTime-startWaitTime)); } - private void modifyTable(Admin admin, String dataTableFullName, HashSet<String> indexes) - throws IOException { - if (upgrade) { - modifyIndexTable(admin, indexes); - modifyDataTable(admin, dataTableFullName); - } else { - modifyDataTable(admin, dataTableFullName); - modifyIndexTable(admin, indexes); - } - } - private void disableTable(Admin admin, String dataTable, HashSet<String>indexes) throws IOException { if (admin.isTableEnabled(TableName.valueOf(dataTable))) { @@ -561,7 +589,24 @@ public class IndexUpgradeTool extends Configured implements Tool { } } - private void enableTable(Admin admin, String dataTable, HashSet<String>indexes) + private void modifyTable(Admin admin, String dataTableFullName, HashSet<String> indexes) + throws IOException { + if (upgrade) { + modifyIndexTable(admin, indexes); + modifyDataTable(admin, dataTableFullName); + if (test && failUpgradeTask) { + throw new RuntimeException("Test requested upgrade failure"); + } + } else { + modifyDataTable(admin, dataTableFullName); + modifyIndexTable(admin, indexes); + if (test && failDowngradeTask) { + throw new RuntimeException("Test requested downgrade failure"); + } + } + } + + private void enableTable(Admin admin, String dataTable, Set<String>indexes) throws IOException { if (!admin.isTableEnabled(TableName.valueOf(dataTable))) { if (!dryRun) { @@ -595,7 +640,8 @@ public class IndexUpgradeTool extends Configured implements Tool { private void rebuildIndexes(Connection conn, Configuration conf, String dataTableFullName) { try { - HashMap<String, IndexInfo> rebuildMap = prepareToRebuildIndexes(conn, dataTableFullName); + HashMap<String, IndexInfo> + rebuildMap = prepareToRebuildIndexes(conn, dataTableFullName); //for rebuilding indexes in case of upgrade and if there are indexes on the table/view. if (rebuildMap.isEmpty()) { @@ -791,7 +837,6 @@ public class IndexUpgradeTool extends Configured implements Tool { String viewSql = getViewSql(tableName, schemaName); ResultSet rs = conn.createStatement().executeQuery(viewSql); - while (rs.next()) { String viewFullName = rs.getString(1); String viewName = SchemaUtil.getTableNameFromFullName(viewFullName);