You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by sk...@apache.org on 2020/04/06 20:16:56 UTC
[phoenix] branch 4.x updated: PHOENIX-5798 IndexUpgrade tool
command line improvements
This is an automated email from the ASF dual-hosted git repository.
skadam pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new ff77018 PHOENIX-5798 IndexUpgrade tool command line improvements
ff77018 is described below
commit ff77018d6b09497d1f30d3018ff58b4660b7c59f
Author: Tanuj Khurana <kh...@gmail.com>
AuthorDate: Tue Mar 24 07:08:10 2020 -0700
PHOENIX-5798 IndexUpgrade tool command line improvements
1. Add an option to IndexUpgrade tool to optionally rebuild indexes. By default, rebuilding is skipped.
2. Add a placeholder option to IndexUpgrade tool to passthrough the option value to the IndexTool
Signed-off-by: s.kadam <s....@apache.org>
---
.../end2end/ParameterizedIndexUpgradeToolIT.java | 19 +++--
.../phoenix/mapreduce/index/IndexUpgradeTool.java | 64 +++++++++------
.../apache/phoenix/index/IndexUpgradeToolTest.java | 95 +++++++++++++++++++---
3 files changed, 133 insertions(+), 45 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 099a2c6..89e9fa9 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
@@ -98,6 +98,7 @@ public class ParameterizedIndexUpgradeToolIT extends BaseTest {
private final boolean mutable;
private final boolean upgrade;
private final boolean isNamespaceEnabled;
+ private final boolean rebuild;
private StringBuilder optionsBuilder;
private String tableDDLOptions;
@@ -136,7 +137,7 @@ public class ParameterizedIndexUpgradeToolIT extends BaseTest {
admin = queryServices.getAdmin();
iut = new IndexUpgradeTool(upgrade ? UPGRADE_OP : ROLLBACK_OP, INPUT_LIST,
null, "/tmp/index_upgrade_" + UUID.randomUUID().toString(),
- true, indexToolMock);
+ true, indexToolMock, rebuild);
iut.setConf(getUtility().getConfiguration());
iut.setTest(true);
if (!mutable) {
@@ -299,20 +300,22 @@ public class ParameterizedIndexUpgradeToolIT extends BaseTest {
}
}
- @Parameters(name ="IndexUpgradeToolIT_mutable={0},upgrade={1},isNamespaceEnabled={2}")
+ @Parameters(name ="IndexUpgradeToolIT_mutable={0},upgrade={1},isNamespaceEnabled={2}, rebuild={3}")
public static synchronized Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
- {false, false, true},
- {true, false, false},
- {false, true, false},
- {true, true, true}
+ {false, false, true, false},
+ {true, false, false, true},
+ {false, true, false, false},
+ {true, true, true, true}
});
}
- public ParameterizedIndexUpgradeToolIT(boolean mutable, boolean upgrade, boolean isNamespaceEnabled) {
+ public ParameterizedIndexUpgradeToolIT(boolean mutable, boolean upgrade,
+ boolean isNamespaceEnabled, boolean rebuild) {
this.mutable = mutable;
this.upgrade = upgrade;
this.isNamespaceEnabled = isNamespaceEnabled;
+ this.rebuild = rebuild;
}
@Test
@@ -332,7 +335,7 @@ public class ParameterizedIndexUpgradeToolIT extends BaseTest {
Assert.assertEquals("Index upgrade tool waited for client cache to expire "
+ "for mutable tables", false, iut.getIsWaitComplete());
}
- if(upgrade) {
+ if(upgrade && rebuild) {
//verifying if index tool was started
Mockito.verify(indexToolMock,
times(11)) // for every index/view-index except index on transaction table
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 e81fcb3..322a32f 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
@@ -83,6 +83,9 @@ public class IndexUpgradeTool extends Configured implements Tool {
private static final Logger LOGGER = Logger.getLogger(IndexUpgradeTool.class.getName());
+ private static final String INDEX_REBUILD_OPTION_SHORT_OPT = "rb";
+ private static final String INDEX_TOOL_OPTION_SHORT_OPT = "tool";
+
private static final Option OPERATION_OPTION = new Option("o", "operation",
true,
"[Required] Operation to perform (upgrade/rollback)");
@@ -99,16 +102,16 @@ public class IndexUpgradeTool extends Configured implements Tool {
private static final Option LOG_FILE_OPTION = new Option("lf", "logfile",
true,
"[Optional] Log file path where the logs are written");
- private static final Option INDEX_SYNC_REBUILD_OPTION = new Option("sr",
- "index-sync-rebuild",
+ private static final Option INDEX_REBUILD_OPTION = new Option(INDEX_REBUILD_OPTION_SHORT_OPT,
+ "index-rebuild",
false,
- "[Optional] Whether or not synchronously rebuild the indexes; "
- + "default rebuild asynchronous");
-
- private static final Option INDEX_VERIFY_OPTION = new Option("v",
- "verify",
+ "[Optional] Rebuild the indexes. Set -" + INDEX_TOOL_OPTION_SHORT_OPT +
+ " to pass options to IndexTool.");
+ private static final Option INDEX_TOOL_OPTION = new Option(INDEX_TOOL_OPTION_SHORT_OPT,
+ "index-tool",
true,
- "[Optional] mode to run indexTool with verify options");
+ "[Optional] Options to pass to indexTool when rebuilding indexes. " +
+ "Set -" + INDEX_REBUILD_OPTION_SHORT_OPT + " to rebuild the index.");
public static final String UPGRADE_OP = "upgrade";
public static final String ROLLBACK_OP = "rollback";
@@ -119,12 +122,12 @@ public class IndexUpgradeTool extends Configured implements Tool {
private HashMap<String, String> prop = new HashMap<>();
private HashMap<String, String> emptyProp = new HashMap<>();
- private boolean dryRun, upgrade, syncRebuild;
+ private boolean dryRun, upgrade, rebuild;
private String operation;
private String inputTables;
private String logFile;
private String inputFile;
- private String verify;
+ private String indexToolOpts;
private boolean test = false;
private boolean isWaitComplete = false;
@@ -151,8 +154,6 @@ public class IndexUpgradeTool extends Configured implements Tool {
public boolean getDryRun() { return this.dryRun; }
- public String getVerify() { return this.verify; }
-
public String getInputTables() {
return this.inputTables;
}
@@ -165,14 +166,19 @@ public class IndexUpgradeTool extends Configured implements Tool {
return this.operation;
}
+ public boolean getIsRebuild() { return this.rebuild; }
+
+ public String getIndexToolOpts() { return this.indexToolOpts; }
+
public IndexUpgradeTool(String mode, String tables, String inputFile,
- String outputFile, boolean dryRun, IndexTool indexTool) {
+ String outputFile, boolean dryRun, IndexTool indexTool, boolean rebuild) {
this.operation = mode;
this.inputTables = tables;
this.inputFile = inputFile;
this.logFile = outputFile;
this.dryRun = dryRun;
this.indexingTool = indexTool;
+ this.rebuild = rebuild;
}
public IndexUpgradeTool () { }
@@ -234,6 +240,11 @@ public class IndexUpgradeTool extends Configured implements Tool {
+TABLE_OPTION.getLongOpt() + " and " + TABLE_CSV_FILE_OPTION.getLongOpt()
+ "; specify only one.");
}
+ if ((cmdLine.hasOption(INDEX_TOOL_OPTION.getOpt()))
+ && !cmdLine.hasOption(INDEX_REBUILD_OPTION.getOpt())) {
+ throw new IllegalStateException("Index tool options should be passed in with "
+ + INDEX_REBUILD_OPTION.getLongOpt());
+ }
return cmdLine;
}
@@ -260,10 +271,10 @@ public class IndexUpgradeTool extends Configured implements Tool {
LOG_FILE_OPTION.setOptionalArg(true);
options.addOption(LOG_FILE_OPTION);
options.addOption(HELP_OPTION);
- INDEX_SYNC_REBUILD_OPTION.setOptionalArg(true);
- options.addOption(INDEX_SYNC_REBUILD_OPTION);
- INDEX_VERIFY_OPTION.setOptionalArg(true);
- options.addOption(INDEX_VERIFY_OPTION);
+ INDEX_REBUILD_OPTION.setOptionalArg(true);
+ options.addOption(INDEX_REBUILD_OPTION);
+ INDEX_TOOL_OPTION.setOptionalArg(true);
+ options.addOption(INDEX_TOOL_OPTION);
return options;
}
@@ -274,8 +285,8 @@ public class IndexUpgradeTool extends Configured implements Tool {
logFile = cmdLine.getOptionValue(LOG_FILE_OPTION.getOpt());
inputFile = cmdLine.getOptionValue(TABLE_CSV_FILE_OPTION.getOpt());
dryRun = cmdLine.hasOption(DRY_RUN_OPTION.getOpt());
- syncRebuild = cmdLine.hasOption(INDEX_SYNC_REBUILD_OPTION.getOpt());
- verify = cmdLine.getOptionValue(INDEX_VERIFY_OPTION.getOpt());
+ rebuild = cmdLine.hasOption(INDEX_REBUILD_OPTION.getOpt());
+ indexToolOpts = cmdLine.getOptionValue(INDEX_TOOL_OPTION.getOpt());
}
@VisibleForTesting
@@ -573,9 +584,10 @@ public class IndexUpgradeTool extends Configured implements Tool {
}
private void rebuildIndexes(Connection conn, Configuration conf, ArrayList<String> tableList) {
- if (!upgrade) {
+ if (!upgrade || !rebuild) {
return;
}
+
for (String table: tableList) {
rebuildIndexes(conn, conf, table);
}
@@ -710,12 +722,12 @@ public class IndexUpgradeTool extends Configured implements Tool {
list.add("-tenant");
list.add(tenantId);
}
- if (syncRebuild) {
- list.add("-runfg");
- }
- if(!Strings.isNullOrEmpty(verify)) {
- list.add("-v");
- list.add(verify);
+
+ if (!Strings.isNullOrEmpty(indexToolOpts)) {
+ String[] options = indexToolOpts.split("\\s+");
+ for (String opt : options) {
+ list.add(opt);
+ }
}
return list.toArray(new String[list.size()]);
}
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/index/IndexUpgradeToolTest.java b/phoenix-core/src/test/java/org/apache/phoenix/index/IndexUpgradeToolTest.java
index 9554aff..a87a4e0 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/index/IndexUpgradeToolTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/index/IndexUpgradeToolTest.java
@@ -31,13 +31,13 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseConfiguration;
import org.apache.hadoop.hbase.HConstants;
+import org.apache.phoenix.mapreduce.index.IndexTool;
import org.apache.phoenix.mapreduce.index.IndexUpgradeTool;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.query.QueryServicesOptions;
import org.apache.phoenix.util.PhoenixRuntime;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -50,18 +50,16 @@ public class IndexUpgradeToolTest {
private final boolean upgrade;
private static final String DUMMY_STRING_VALUE = "anyValue";
private static final String DUMMY_VERIFY_VALUE = "someVerifyValue";
+ private static final String ONLY_VERIFY_VALUE = "ONLY";
private IndexUpgradeTool indexUpgradeTool=null;
private String outputFile;
public IndexUpgradeToolTest(boolean upgrade) {
this.upgrade = upgrade;
+ this.outputFile = "/tmp/index_upgrade_" + UUID.randomUUID().toString();
}
- @Before
- public void setup() {
- outputFile = "/tmp/index_upgrade_" + UUID.randomUUID().toString();
- String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb",
- INPUT_LIST, "-lf", outputFile, "-d", "-v", DUMMY_VERIFY_VALUE};
+ private void setup(String[] args) {
indexUpgradeTool = new IndexUpgradeTool();
CommandLine cmd = indexUpgradeTool.parseOptions(args);
indexUpgradeTool.initializeTool(cmd);
@@ -69,26 +67,101 @@ public class IndexUpgradeToolTest {
@Test
public void testCommandLineParsing() {
+ String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb",
+ INPUT_LIST, "-lf", outputFile, "-d"};
+ setup(args);
Assert.assertEquals(indexUpgradeTool.getDryRun(),true);
Assert.assertEquals(indexUpgradeTool.getInputTables(), INPUT_LIST);
Assert.assertEquals(indexUpgradeTool.getOperation(), upgrade ? UPGRADE_OP : ROLLBACK_OP);
Assert.assertEquals(indexUpgradeTool.getLogFile(), outputFile);
+ // verify index rebuild is disabled by default
+ Assert.assertEquals(false, indexUpgradeTool.getIsRebuild());
+ Assert.assertNull(indexUpgradeTool.getIndexToolOpts());
}
@Test
- public void testIfVerifyOptionIsPassedToTool() {
+ public void testRebuildOptionParsing() {
+ String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb",
+ INPUT_LIST, "-rb"};
+ setup(args);
+ Assert.assertEquals(true, indexUpgradeTool.getIsRebuild());
+ Assert.assertNull(indexUpgradeTool.getIndexToolOpts());
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testIndexToolOptionsNoRebuild() {
+ String indexToolOpts = "-v " + DUMMY_VERIFY_VALUE;
+ String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb", INPUT_LIST,
+ "-tool", indexToolOpts};
+ setup(args);
+ }
+
+ @Test
+ public void testIfOptionsArePassedToIndexTool() {
if (!upgrade) {
return;
}
- Assert.assertEquals("value passed with verify option does not match with provided value",
- DUMMY_VERIFY_VALUE, indexUpgradeTool.getVerify());
+ String [] indexToolOpts = {"-v", ONLY_VERIFY_VALUE, "-runfg", "-st", "100"};
+ String indexToolarg = String.join(" ", indexToolOpts);
+ String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb",
+ INPUT_LIST, "-lf", outputFile, "-d", "-rb", "-tool", indexToolarg };
+ setup(args);
+
+ Assert.assertEquals("value passed to index tool option does not match with provided value",
+ indexToolarg, indexUpgradeTool.getIndexToolOpts());
String [] values = indexUpgradeTool.getIndexToolArgValues(DUMMY_STRING_VALUE,
DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE);
List<String> argList = Arrays.asList(values);
- Assert.assertTrue(argList.contains(DUMMY_VERIFY_VALUE));
Assert.assertTrue(argList.contains("-v"));
+ Assert.assertTrue(argList.contains(ONLY_VERIFY_VALUE));
Assert.assertEquals("verify option and value are not passed consecutively", 1,
- argList.indexOf(DUMMY_VERIFY_VALUE) - argList.indexOf("-v"));
+ argList.indexOf(ONLY_VERIFY_VALUE) - argList.indexOf("-v"));
+ Assert.assertTrue(argList.contains("-runfg"));
+ Assert.assertTrue(argList.contains("-st"));
+
+ // ensure that index tool can parse the options and raises no exceptions
+ IndexTool it = new IndexTool();
+ CommandLine commandLine = it.parseOptions(values);
+ it.populateIndexToolAttributes(commandLine);
+ }
+
+ @Test
+ public void testMalformedSpacingOptionsArePassedToIndexTool() {
+ if (!upgrade) {
+ return;
+ }
+ String [] indexToolOpts = {"-v"+ONLY_VERIFY_VALUE, " -runfg", " -st ", "100 "};
+ String indexToolarg = String.join(" ", indexToolOpts);
+ String [] args = {"-o", upgrade ? UPGRADE_OP : ROLLBACK_OP, "-tb",
+ INPUT_LIST, "-rb", "-tool", indexToolarg };
+ setup(args);
+
+ Assert.assertEquals("value passed to index tool option does not match with provided value",
+ indexToolarg, indexUpgradeTool.getIndexToolOpts());
+ String [] values = indexUpgradeTool.getIndexToolArgValues(DUMMY_STRING_VALUE,
+ DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE);
+ List<String> argList = Arrays.asList(values);
+ Assert.assertTrue(argList.contains("-v" + ONLY_VERIFY_VALUE));
+ Assert.assertTrue(argList.contains("-runfg"));
+ Assert.assertTrue(argList.contains("-st"));
+
+ // ensure that index tool can parse the options and raises no exceptions
+ IndexTool it = new IndexTool();
+ CommandLine commandLine = it.parseOptions(values);
+ it.populateIndexToolAttributes(commandLine);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testBadIndexToolOptions() {
+ String [] indexToolOpts = {"-v" + DUMMY_VERIFY_VALUE};
+ String indexToolarg = String.join(" ", indexToolOpts);
+ String [] args = {"-o", UPGRADE_OP, "-tb", INPUT_LIST, "-rb", "-tool", indexToolarg };
+ setup(args);
+ String [] values = indexUpgradeTool.getIndexToolArgValues(DUMMY_STRING_VALUE,
+ DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE, DUMMY_STRING_VALUE);
+ IndexTool it = new IndexTool();
+ CommandLine commandLine = it.parseOptions(values);
+ it.populateIndexToolAttributes(commandLine);
}
@Parameters(name ="IndexUpgradeToolTest_mutable={1}")