You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by el...@apache.org on 2020/05/28 23:22:50 UTC
[phoenix] branch master updated: Revert "PHOENIX-5922 -
IndexUpgradeTool should always re-enable tables on failure"
This is an automated email from the ASF dual-hosted git repository.
elserj 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 18b219d Revert "PHOENIX-5922 - IndexUpgradeTool should always re-enable tables on failure"
18b219d is described below
commit 18b219d7d270d546d7be29164229161531bd9330
Author: Josh Elser <el...@apache.org>
AuthorDate: Thu May 28 19:18:34 2020 -0400
Revert "PHOENIX-5922 - IndexUpgradeTool should always re-enable tables on failure"
This reverts commit 45929b5a74feddb647571c7d7c057142973b45b0.
---
.../end2end/ParameterizedIndexUpgradeToolIT.java | 46 ---------
.../phoenix/mapreduce/index/IndexUpgradeTool.java | 107 ++++++---------------
2 files changed, 31 insertions(+), 122 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 2ed249e..5479576 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,52 +381,6 @@ 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 fca7930..a856b5e 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,8 +26,6 @@ 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;
@@ -62,8 +60,6 @@ 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;
@@ -135,10 +131,6 @@ 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;
@@ -178,15 +170,6 @@ 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;
@@ -212,12 +195,7 @@ public class IndexUpgradeTool extends Configured implements Tool {
initializeTool(cmdLine);
prepareToolSetup();
executeTool();
- if (hasFailure) {
- return -1;
- } else {
- return 0;
- }
-
+ return 0;
}
/**
@@ -231,7 +209,7 @@ public class IndexUpgradeTool extends Configured implements Tool {
final Options options = getOptions();
- CommandLineParser parser = new PosixParser();
+ CommandLineParser parser = new DefaultParser();
CommandLine cmdLine = null;
try {
cmdLine = parser.parse(options, args);
@@ -343,7 +321,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);
}
}
@@ -420,20 +398,15 @@ public class IndexUpgradeTool extends Configured implements Tool {
executeToolForMutableTables(conn, queryServices, conf, mutableList);
enableImmutableTables(queryServices, immutableList, startWaitTime);
rebuildIndexes(conn, conf, immutableList);
- if (hasFailure) {
- return -1;
- } else {
- return 0;
- }
+ return 0;
}
private long executeToolForImmutableTables(ConnectionQueryServices queryServices,
- List<String> immutableList) {
+ ArrayList<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);
@@ -441,13 +414,12 @@ public class IndexUpgradeTool extends Configured implements Tool {
+ " (immutable)");
disableTable(admin, dataTableFullName, indexes);
modifyTable(admin, dataTableFullName, indexes);
- } catch (Throwable e) {
+ } catch (IOException | SQLException e) {
LOGGER.severe("Something went wrong while disabling "
+ "or modifying immutable table " + e);
- handleFailure(queryServices, dataTableFullName, immutableList, failedTables);
+ handleFailure(queryServices, dataTableFullName, immutableList);
}
}
- immutableList.removeAll(failedTables);
long startWaitTime = EnvironmentEdgeManager.currentTimeMillis();
return startWaitTime;
}
@@ -460,7 +432,6 @@ 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);
@@ -469,22 +440,19 @@ public class IndexUpgradeTool extends Configured implements Tool {
modifyTable(admin, dataTableFullName, indexes);
enableTable(admin, dataTableFullName, indexes);
LOGGER.info("Completed " + operation + " of " + dataTableFullName);
- } catch (Throwable e) {
+ } catch (IOException | SQLException e) {
LOGGER.severe("Something went wrong while executing "
- + operation + " steps for "+ dataTableFullName + " " + e);
- handleFailure(queryServices, dataTableFullName, mutableTables, failedTables);
+ + operation + " steps for "+ dataTableFullName + " " + e);
+ handleFailure(queryServices, dataTableFullName, mutableTables);
}
}
- mutableTables.removeAll(failedTables);
// Opportunistically kick-off index rebuilds after upgrade operation
rebuildIndexes(conn, conf, mutableTables);
}
private void handleFailure(ConnectionQueryServices queryServices,
String dataTableFullName,
- List<String> tableList,
- List<String> failedTables) {
- hasFailure = true;
+ ArrayList<String> tableList) {
LOGGER.info("Performing error handling to revert the steps taken during " + operation);
HashSet<String> indexes = tablesAndIndexes.get(dataTableFullName);
try (Admin admin = queryServices.getAdmin()) {
@@ -495,21 +463,14 @@ public class IndexUpgradeTool extends Configured implements Tool {
upgrade = !upgrade;
tablesAndIndexes.remove(dataTableFullName); //removing from the map
- failedTables.add(dataTableFullName); //everything in failed tables will later be
- // removed from the list
+ tableList.remove(dataTableFullName); //removing from the list
LOGGER.severe(dataTableFullName+" has been removed from the list as tool failed"
+ " to perform "+operation);
- } catch (Throwable e) {
+ } catch (IOException | SQLException e) {
LOGGER.severe("Revert of the "+operation +" failed in error handling, "
- + "re-enabling tables and then throwing runtime exception");
+ + "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);
}
}
@@ -553,9 +514,9 @@ public class IndexUpgradeTool extends Configured implements Tool {
}
}
- private String getSubListString(List<String> tableList, String dataTableFullName) {
- return StringUtils.join(tableList.subList(tableList.indexOf(dataTableFullName),
- tableList.size()), ",");
+ private String getSubListString(ArrayList<String> tableList, String dataTableFullName) {
+ return StringUtils.join(",", tableList.subList(tableList.indexOf(dataTableFullName),
+ tableList.size()));
}
private long getWaitMoreTime(long startWaitTime) {
@@ -567,6 +528,17 @@ 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))) {
@@ -589,24 +561,7 @@ public class IndexUpgradeTool extends Configured implements Tool {
}
}
- 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)
+ private void enableTable(Admin admin, String dataTable, HashSet<String>indexes)
throws IOException {
if (!admin.isTableEnabled(TableName.valueOf(dataTable))) {
if (!dryRun) {
@@ -640,8 +595,7 @@ 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()) {
@@ -837,6 +791,7 @@ 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);