This is an automated email from the ASF dual-hosted git repository.
skadam 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 bb1db52 PHOENIX-5798 IndexUpgrade tool command line improvements
bb1db52 is described below
commit bb1db520d0be1a19f1c62c2a0fb78aaad1d30ac4
Author: Tanuj Khurana <khurana.ta...@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.ka...@apache.org>
---
.../end2end/ParameterizedIndexUpgradeToolIT.java | 19 +++--
.../phoenix/mapreduce/index/IndexUpgradeTool.java | 65 +++++++++------
.../apache/phoenix/index/IndexUpgradeToolTest.java | 95 +++++++++++++++++++---
3 files changed, 133 insertions(+), 46 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 bdaa2ac..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
@@ -97,6 +97,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;
@@ -135,7 +136,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) {
@@ -298,20 +299,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
@@ -331,7 +334,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 7ed24dc..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
@@ -82,6 +82,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)");
@@ -98,16 +101,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,13 +122,13 @@ 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 boolean isWaitComplete = false;
- private String verify;
+ private String indexToolOpts;
private boolean test = false;
@@ -149,11 +152,8 @@ public class IndexUpgradeTool extends Configured
implements Tool {
public boolean getIsWaitComplete() { return this.isWaitComplete; }
-
public boolean getDryRun() { return this.dryRun; }
- public String getVerify() { return this.verify; }
-
public String getInputTables() {
return this.inputTables;
}
@@ -166,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 () { }
@@ -235,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;
}
@@ -261,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;
}
@@ -275,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
@@ -574,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);
}
@@ -707,12 +718,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}")
