huaxiangsun commented on a change in pull request #62:
URL:
https://github.com/apache/hbase-operator-tools/pull/62#discussion_r437045746
##########
File path: hbase-hbck2/README.md
##########
@@ -124,17 +124,20 @@ Command:
SEE ALSO: reportMissingRegionsInMeta
SEE ALSO: fixMeta
- assigns [OPTIONS] <ENCODED_REGIONNAME>...
+ assigns [OPTIONS] <ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...
Options:
-o,--override override ownership by another procedure
+ -i,--inputFiles take one or more encoded region names
A 'raw' assign that can be used even during Master initialization (if
the -skip flag is specified). Skirts Coprocessors. Pass one or more
encoded region names. 1588230740 is the hard-coded name for the
hbase:meta region and de00010733901a05f5a2a3a382e27dd4 is an example of
what a user-space encoded region name looks like. For example:
- $ HBCK2 assign 1588230740 de00010733901a05f5a2a3a382e27dd4
+ $ HBCK2 assigns 1588230740 de00010733901a05f5a2a3a382e27dd4
Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
-
+ If -i or --inputFiles is specified, pass one or more input file names.
Review comment:
Are multiple input file too much? Maybe one file is good enough.
##########
File path: hbase-hbck2/README.md
##########
@@ -124,17 +124,20 @@ Command:
SEE ALSO: reportMissingRegionsInMeta
SEE ALSO: fixMeta
- assigns [OPTIONS] <ENCODED_REGIONNAME>...
+ assigns [OPTIONS] <ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...
Options:
-o,--override override ownership by another procedure
+ -i,--inputFiles take one or more encoded region names
A 'raw' assign that can be used even during Master initialization (if
the -skip flag is specified). Skirts Coprocessors. Pass one or more
encoded region names. 1588230740 is the hard-coded name for the
hbase:meta region and de00010733901a05f5a2a3a382e27dd4 is an example of
what a user-space encoded region name looks like. For example:
- $ HBCK2 assign 1588230740 de00010733901a05f5a2a3a382e27dd4
+ $ HBCK2 assigns 1588230740 de00010733901a05f5a2a3a382e27dd4
Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
-
+ If -i or --inputFiles is specified, pass one or more input file names.
Review comment:
Read again, the code only handles one input file, we can clean up the
comments/help message accordingly.
##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -147,6 +149,27 @@ public void testAssigns() throws IOException {
LOG.info("RS: {}", rs.toString());
assertTrue(rs.toString(), rs.isOpened());
}
+ // test input files
Review comment:
Can we have test similar to the following? That will really go through
the option processing path.
The output will have a set of pids and we wait for pids to check the regions
are opened.
```
private String testFormatExtraRegionsInMeta(String[] args) throws
IOException {
HBCK2 hbck = new HBCK2(TEST_UTIL.getConfiguration());
final StringBuilder builder = new StringBuilder();
PrintStream originalOS = System.out;
OutputStream testOS = new OutputStream() {
@Override public void write(int b) throws IOException {
builder.append((char)b);
}
};
System.setOut(new PrintStream(testOS));
hbck.run(args);
System.setOut(originalOS);
return builder.toString();
```
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -294,7 +299,29 @@ int setRegionState(ClusterConnection connection, String
region,
return null;
}
boolean overrideFlag = commandLine.hasOption(override.getOpt());
- return hbck.assigns(commandLine.getArgList(), overrideFlag);
+
+ List<String> argList = commandLine.getArgList();
+ if (!commandLine.hasOption(inputFile.getOpt())) {
+ return hbck.assigns(argList, overrideFlag);
+ } else {
+ List<String> assignmentList = new ArrayList<>();
+ for (String filePath : argList) {
Review comment:
Got it, thanks.
##########
File path: hbase-hbck2/README.md
##########
@@ -124,17 +124,20 @@ Command:
SEE ALSO: reportMissingRegionsInMeta
SEE ALSO: fixMeta
- assigns [OPTIONS] <ENCODED_REGIONNAME>...
+ assigns [OPTIONS] <ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...
Options:
-o,--override override ownership by another procedure
+ -i,--inputFiles take one or more encoded region names
A 'raw' assign that can be used even during Master initialization (if
the -skip flag is specified). Skirts Coprocessors. Pass one or more
encoded region names. 1588230740 is the hard-coded name for the
hbase:meta region and de00010733901a05f5a2a3a382e27dd4 is an example of
what a user-space encoded region name looks like. For example:
- $ HBCK2 assign 1588230740 de00010733901a05f5a2a3a382e27dd4
+ $ HBCK2 assigns 1588230740 de00010733901a05f5a2a3a382e27dd4
Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
-
+ If -i or --inputFiles is specified, pass one or more input file names.
Review comment:
Good.
##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -152,17 +152,11 @@ public void testAssigns() throws IOException {
// test input files
String testFile = "inputForAssignsTest";
FileOutputStream output = new FileOutputStream(testFile, false);
- LOG.info("writing to: {}", testFile);
Review comment:
Regions in regionStrsArray has been assigned already. These regions need
to be unassigned first before testing assigns with file option.
```
List<Long> pids = this.hbck2.unassigns(hbck, regionStrsArray);
waitOnPids(pids);
```
The only missing part from testRunWithArgs is to get pid output from console
and then wait on them.
Another way is to wait until the active procedure count to 0.
```
MasterServices services = TEST_UTIL.getHBaseCluster().getMaster();
while (!services.getMasterProcedureExecutor().getActiveProcIds().isEmpty()) {
sleep (200);
}
```
##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -152,17 +152,11 @@ public void testAssigns() throws IOException {
// test input files
String testFile = "inputForAssignsTest";
FileOutputStream output = new FileOutputStream(testFile, false);
- LOG.info("writing to: {}", testFile);
Review comment:
Regions in regionStrsArray has been assigned already. These regions need
to be unassigned first before testing assigns with file option.
```
List<Long> pids = this.hbck2.unassigns(hbck, regionStrsArray);
waitOnPids(pids);
```
The only missing part from testRunWithArgs is to get pid output from console
and then wait on them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
