You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/09 15:51:24 UTC

[GitHub] [hbase-operator-tools] huaxiangsun commented on a change in pull request #62: HBCK takes one or more files for assigns

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:
users@infra.apache.org