wchevreuil commented on a change in pull request #69:
URL:
https://github.com/apache/hbase-operator-tools/pull/69#discussion_r443493638
##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
of what a userspace encoded region name looks like. For example:
$ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+ If -i or --inputFiles is specified, pass one or more input file names.
Review comment:
We need still to update README for each of the modified commands. For
example, _assigns_ descriptions reads:
`assigns [OPTIONS] <ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...`
But _extraRegionsInMeta_ still reads:
`extraRegionsInMeta <NAMESPACE|NAMESPACE:TABLENAME>...`
However, command usage for modified commands do mention the correct
parameters, so we should make README consistent.
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -357,23 +363,24 @@ int setRegionState(ClusterConnection connection, String
region,
if (commandLine.hasOption(wait.getOpt())) {
lockWait = Integer.parseInt(commandLine.getOptionValue(wait.getOpt()));
}
- String[] pidStrs = commandLine.getArgs();
- if (pidStrs == null || pidStrs.length <= 0) {
+ List<String> pidStrs = getFromArgsOrFiles(commandLine.getArgList());
Review comment:
Requires UT for the new parameter option.
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -228,7 +229,7 @@ int setRegionState(ClusterConnection connection, String
region,
Map<TableName, List<String>> result = new HashMap<>();
try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
new FsRegionsMetaRecoverer(this.conf)) {
- List<String> namespacesTables =
formatNameSpaceTableParam(commandLine.getArgs());
+ List<String> namespacesTables =
getFromArgsOrFiles(formatNameSpaceTableParam(commandLine.getArgs()));
Review comment:
Still missing UT for when files are passed as parameter. See
_TestHBCK2.testFormatFixExtraInMetaOneExtraSpecificTable_.
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -203,8 +204,8 @@ int setRegionState(ClusterConnection connection, String
region,
Map<TableName,List<Path>> report;
try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
new FsRegionsMetaRecoverer(this.conf)) {
- report = fsRegionsMetaRecoverer.reportTablesMissingRegions(
- formatNameSpaceTableParam(nameSpaceOrTable));
+ report =
fsRegionsMetaRecoverer.reportTablesMissingRegions(getFromArgsOrFiles(
Review comment:
Still missing UT for _reportMissingRegionsInMeta_. See
_TestHBCK2.testReportMissingRegionsInMeta_.
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -357,23 +363,24 @@ int setRegionState(ClusterConnection connection, String
region,
if (commandLine.hasOption(wait.getOpt())) {
lockWait = Integer.parseInt(commandLine.getOptionValue(wait.getOpt()));
}
- String[] pidStrs = commandLine.getArgs();
- if (pidStrs == null || pidStrs.length <= 0) {
+ List<String> pidStrs = getFromArgsOrFiles(commandLine.getArgList());
+ if (pidStrs == null || pidStrs.size() <= 0) {
showErrorMessage("No pids supplied.");
return null;
}
boolean overrideFlag = commandLine.hasOption(override.getOpt());
boolean recursiveFlag = commandLine.hasOption(recursive.getOpt());
- List<Long> pids =
Arrays.stream(pidStrs).map(Long::valueOf).collect(Collectors.toList());
+ List<Long> pids =
pidStrs.stream().map(Long::valueOf).collect(Collectors.toList());
try (ClusterConnection connection = connect(); Hbck hbck =
connection.getHbck()) {
checkFunctionSupported(connection, BYPASS);
return hbck.bypassProcedure(pids, lockWait, overrideFlag, recursiveFlag);
}
}
List<Long> scheduleRecoveries(Hbck hbck, String[] args) throws IOException {
+ List<String> arglist = getFromArgsOrFiles(Arrays.asList(args));
Review comment:
Requires UT for the new parameter option.
##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -265,16 +266,12 @@ int setRegionState(ClusterConnection connection, String
region,
return result;
}
- private List<String> formatNameSpaceTableParam(String... nameSpaceOrTable) {
- return nameSpaceOrTable != null ? Arrays.asList(nameSpaceOrTable) : null;
- }
-
List<Future<List<String>>> addMissingRegionsInMetaForTables(String...
nameSpaceOrTable) throws IOException {
try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
new FsRegionsMetaRecoverer(this.conf)) {
- return fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(
- formatNameSpaceTableParam(nameSpaceOrTable));
+ return
fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(getFromArgsOrFiles(
Review comment:
Still missing UT for the condition where files are passed as input. See
_TestHBCK2.testAddMissingRegionsInMetaForTables_
----------------------------------------------------------------
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]
