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 2021/05/26 03:18:10 UTC

[GitHub] [hbase-operator-tools] taklwu opened a new pull request #88: Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

taklwu opened a new pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88


   recently we found a bug that when running `hbck filesystem -f` on a non-HDFS and caused `Wrong FS` error and cannot move forward. 
   
   this change fix this problem and use `FSUtils.getCurrentFileSystem` to respect the root directory of what we set with `hbase.rootdir` 


-- 
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

[GitHub] [hbase-operator-tools] anoopsjohn commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r642383378



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCKFsUtils.java
##########
@@ -107,6 +107,15 @@ public static Path getRootDir(final Configuration c) throws IOException {
     return p.makeQualified(fs.getUri(), fs.getWorkingDirectory());
   }
 
+  /**
+   * @param conf must not be null
+   * @return Returns the filesystem of the hbase rootdir.
+   * @throws IOException from underlying FileSystem
+   */
+  public static FileSystem getCurrentFileSystem(Configuration conf) throws IOException {

Review comment:
       Should we call it getRootFileSystem  ?  For dir, we use rootDir and walDir terms.  Same way.




-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r643404148



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCKFsUtils.java
##########
@@ -107,6 +107,15 @@ public static Path getRootDir(final Configuration c) throws IOException {
     return p.makeQualified(fs.getUri(), fs.getWorkingDirectory());
   }
 
+  /**
+   * @param conf must not be null
+   * @return Returns the filesystem of the hbase rootdir.
+   * @throws IOException from underlying FileSystem
+   */
+  public static FileSystem getCurrentFileSystem(Configuration conf) throws IOException {

Review comment:
       thanks @anoopsjohn , I renamed that to `getRootDirFileSystem`




-- 
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

[GitHub] [hbase-operator-tools] taklwu edited a comment on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu edited a comment on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848429515


   @jojochuang you got me lol, I was wondered if I can create the PR directly but I go back to create a JIRA.


-- 
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

[GitHub] [hbase-operator-tools] taklwu merged pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu merged pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88


   


-- 
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

[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-849365169


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 52s |  master passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  5s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 18s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  5s |  The patch does not generate ASF License warnings.  |
   |  |   |   9m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/4/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/88 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 593992be9bc6 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 33d4c31 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/4/testReport/ |
   | Max. process+thread count | 957 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/4/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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

[GitHub] [hbase-operator-tools] joshelser commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r639381306



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       In master, it looks like FSUtils no longer extends CommonFSUtils and thus, I don't think this would compile. Glancing over branches, I think you could make this `CommonFSUtils.getCurrentFileSystem(Configuration)`




-- 
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

[GitHub] [hbase-operator-tools] taklwu edited a comment on pull request #88: Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu edited a comment on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848424321


   e.g. this is one of the captured ERROR message 
   
   ```
   Exception in thread "main" java.lang.IllegalArgumentException: Wrong FS: s3://xyz/data/default/abc, expected: hdfs://xyz/
           at org.apache.hadoop.fs.FileSystem.checkPath(FileSystem.java:737)
           at org.apache.hadoop.hdfs.DistributedFileSystem.getPathName(DistributedFileSystem.java:233)
           at org.apache.hadoop.hdfs.DistributedFileSystem.listStatusInternal(DistributedFileSystem.java:1045)
           at org.apache.hadoop.hdfs.DistributedFileSystem.access$1000(DistributedFileSystem.java:131)
           at org.apache.hadoop.hdfs.DistributedFileSystem$24.doCall(DistributedFileSystem.java:1112)
           at org.apache.hadoop.hdfs.DistributedFileSystem$24.doCall(DistributedFileSystem.java:1109)
           at org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
           at org.apache.hadoop.hdfs.DistributedFileSystem.listStatus(DistributedFileSystem.java:1119)
           at org.apache.hadoop.hbase.util.FSUtils.listStatusWithStatusFilter(FSUtils.java:1530)
           at org.apache.hbase.hbck1.HFileCorruptionChecker.checkTableDir(HFileCorruptionChecker.java:320)
           at org.apache.hbase.hbck1.HFileCorruptionChecker.checkTables(HFileCorruptionChecker.java:431)
           at org.apache.hbase.FileSystemFsck.fsck(FileSystemFsck.java:87)
           at org.apache.hbase.HBCK2.doCommandLine(HBCK2.java:721)
           at org.apache.hbase.HBCK2.run(HBCK2.java:631)
           at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76)
           at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:90)
           at org.apache.hbase.HBCK2.main(HBCK2.java:865)
   ```


-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on pull request #88: Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848424321


   e.g you this is one of the captured ERROR message 
   
   ```
   Exception in thread "main" java.lang.IllegalArgumentException: Wrong FS: s3://xyz/data/default/abc, expected: hdfs://xyz/
           at org.apache.hadoop.fs.FileSystem.checkPath(FileSystem.java:737)
           at org.apache.hadoop.hdfs.DistributedFileSystem.getPathName(DistributedFileSystem.java:233)
           at org.apache.hadoop.hdfs.DistributedFileSystem.listStatusInternal(DistributedFileSystem.java:1045)
           at org.apache.hadoop.hdfs.DistributedFileSystem.access$1000(DistributedFileSystem.java:131)
           at org.apache.hadoop.hdfs.DistributedFileSystem$24.doCall(DistributedFileSystem.java:1112)
           at org.apache.hadoop.hdfs.DistributedFileSystem$24.doCall(DistributedFileSystem.java:1109)
           at org.apache.hadoop.fs.FileSystemLinkResolver.resolve(FileSystemLinkResolver.java:81)
           at org.apache.hadoop.hdfs.DistributedFileSystem.listStatus(DistributedFileSystem.java:1119)
           at org.apache.hadoop.hbase.util.FSUtils.listStatusWithStatusFilter(FSUtils.java:1530)
           at org.apache.hbase.hbck1.HFileCorruptionChecker.checkTableDir(HFileCorruptionChecker.java:320)
           at org.apache.hbase.hbck1.HFileCorruptionChecker.checkTables(HFileCorruptionChecker.java:431)
           at org.apache.hbase.FileSystemFsck.fsck(FileSystemFsck.java:87)
           at org.apache.hbase.HBCK2.doCommandLine(HBCK2.java:721)
           at org.apache.hbase.HBCK2.run(HBCK2.java:631)
           at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76)
           at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:90)
           at org.apache.hbase.HBCK2.main(HBCK2.java:865)
   ```


-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r640272518



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       @wchevreuil, I changed to use `HBCKFSUtils` for all the place changed in this PR. 




-- 
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

[GitHub] [hbase-operator-tools] wchevreuil commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r639534992



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       We should refrain from using hbase's `InterfaceAudience.Private` classes such as `FSUtils` in operator tools. We had problems in the past related to compatibility breaks. For that reason we had created `HBCKFSUtils` in operator tools. Please use this `HBCKFSUtils`, if the method you are depending on doesn't yet exist, please implement it there (or simply copy it from `FSUtils` there).




-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r640115793



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       good to know about use `CommonFSUtils` and further `HBCKFSUtils`. 
   
   for whatever I touch here, I can of coz use/implement `HBCKFSUtils`, but again.....I felt like we should not re-implement the same thing across `hbase-common` and `hbase-operator-tools` (sorry I'm a bit confused) So, for this change, should we use Utilities class of `CommonFSUtils` and further make it `InterfaceAudience.Public` (sorry that I just don't like copying code)? 
   
   I'm learning, IMO the standard of using `InterfaceAudience.Private` outside of hbase core is a bit higher that if we think we should not use any `InterfaceAudience.Private` classes in `hbase-common` or `hbase-common` itself should not be used in `hbase-operator-tools`, we may need to have a larger refactoring to copy/implement few more functions from `CommonFSUtils` and `FSUtils` to `HBCKFSUtils`. however, to limit the change/scope of this bug fix and I found more classes in this package make use of `CommonFSUtils` and `FSUtils`, I would propose and suggest to have a follow-up PR if needed. 
   
   




-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-849308745


   @joshelser I added unit tests to make sure we use the filesystem defined in `hbase.rootdir` instead of `fs.defaultFS`.  if you're looking for end-to-end unit tests for `HBaseFsck` (currently does not exist for any functions in that class), I'd like to create a separate JIRAs for those are being used by `HBCK2` and limit the scope of this PR.
   
   @z-york , I did that in IDE and but here is what you request for a proof. 
   
   * without my change, we found 13 lines with `FileSystem.get`, 2 lines in README is stacktrace and is not related to this change  
   
   ```
   $ git checkout rel/1.1.0
   $ grep -R "^.*\bFileSystem\.get" .
   ./README.md:        at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2799)
   ./README.md:        at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:389)
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptors.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptors.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptors.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptorForceCreation.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptorForceCreation.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/test/java/org/apache/hbase/TestHBCKFsTableDescriptorForceCreation.java:    FileSystem fs = FileSystem.get(UTIL.getConfiguration());
   ./src/main/java/org/apache/hbase/hbck1/HBaseFsck.java:    FileSystem fs = FileSystem.get(getConf());
   ./src/main/java/org/apache/hbase/hbck1/HBaseFsck.java:          CommonFSUtils.delete(FileSystem.get(getConf()), waldir, true));
   ./src/main/java/org/apache/hbase/hbck1/HBaseFsck.java:        FileSystem fs = FileSystem.get(conf);
   ./src/main/java/org/apache/hbase/hbck1/HBaseFsck.java:        FileSystem fs = FileSystem.get(conf);
   ./src/main/java/org/apache/hbase/hbck1/HFileCorruptionChecker.java:    this.fs = FileSystem.get(conf);
   
   $ grep -R "^.*\bFileSystem\.get" . | wc -l
         13
   	  
   ```
   
   * with my change, we don't find `FileSystem.get` in any java classes   
   
   ```
   $ grep -R "^.*\bFileSystem\.get" .
   ./README.md:        at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2799)
   ./README.md:        at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:389)
   $ grep -R "^.*\bFileSystem\.get" . | wc -l
          2
   ```	


-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848429515


   @jojochuang you got me lol, I was wondered if I can create directly but I go back to create a JIRA.


-- 
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

[GitHub] [hbase-operator-tools] z-york commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
z-york commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848957542


   +1 to the other changes requested, can you also do a grep of hbck to see if there are any other places being missed? This is definitely something we don't want to have broken. It would be a pain trying to handle a production issue and your tool to fix the inconsistency won't work :( (I know you wouldn't know anything about that :P )


-- 
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

[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #88: Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848426118


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  1s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 46s |  master passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  8s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  8s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   4m 34s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 26s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/1/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/88 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 7cc0cbb4d9d4 5.4.0-1043-aws #45~18.04.1-Ubuntu SMP Fri Apr 9 23:32:25 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 33d4c31 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/1/testReport/ |
   | Max. process+thread count | 951 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/1/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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

[GitHub] [hbase-operator-tools] anoopsjohn commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r643642855



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1927,7 +1931,7 @@ public boolean rebuildMeta() throws IOException, InterruptedException {
       HBaseTestingUtility.closeRegionAndWAL(meta);
       // Clean out the WAL we created and used here.
       LOG.info("Deleting {}, result={}", waldir,
-          CommonFSUtils.delete(FileSystem.get(getConf()), waldir, true));
+          HBCKFsUtils.delete(waldir.getFileSystem(getConf()), waldir, true));

Review comment:
       Not on this patch. But here is an issue. The actual delete() call happening within a Log statement. If INFO level not enabled, the delete wont get called.  Can we fix that also here pls?




-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r640115793



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       good to know about use `CommonFSUtils` and further `HBCKFSUtils`. 
   
   for whatever I touch here, I can of coz use/implement `HBCKFSUtils`, but again.....I felt like we should not re-implement the same thing across `hbase-common` and `hbase-operator-tools` (sorry I'm a bit confused even I understand about the compatibility across two repositories/projects) So, for this change, should we use Utilities class of `CommonFSUtils` and further make it `InterfaceAudience.Public` (sorry that I just don't like copying code)? 
   
   I'm learning, IMO the standard of using `InterfaceAudience.Private` outside of hbase core is a bit higher that if we think we should not use any `InterfaceAudience.Private` classes in `hbase-common` or `hbase-common` itself should not be used in `hbase-operator-tools`, we may need to have a larger refactoring to copy/implement few more functions from `CommonFSUtils` and `FSUtils` to `HBCKFSUtils`. however, to limit the change/scope of this bug fix and I found more classes in this package make use of `CommonFSUtils` and `FSUtils`, I would propose and suggest to have a follow-up PR if needed. 
   
   




-- 
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

[GitHub] [hbase-operator-tools] taklwu commented on a change in pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#discussion_r640115793



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/hbck1/HBaseFsck.java
##########
@@ -1535,7 +1535,7 @@ private void loadHdfsRegioninfo(HbckInfo hbi) throws IOException {
       return;
     }
 
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = FSUtils.getCurrentFileSystem(getConf());

Review comment:
       good to know, I have updated to use `CommonFSUtils` and further `HBCKFSUtils`. 
   
   for whatever I touch here, I can of coz use/implement `HBCKFSUtils`, but again.....I felt like we should not re-implement the same thing across `hbase-common` and `hbase-operator-tools` (sorry I'm a bit confused) So, for this change, should we use Utilities class of `CommonFSUtils` and further make it `InterfaceAudience.Public` (sorry that I just don't like copying code)? 
   
   I'm learning, IMO the standard of using `InterfaceAudience.Private` outside of hbase core is a bit higher that if we think we should not use any `InterfaceAudience.Private` classes in `hbase-common` or `hbase-common` itself should not be used in `hbase-operator-tools`, we may need to have a larger refactoring to copy/implement few more functions from `CommonFSUtils` and `FSUtils` to `HBCKFSUtils`. however, to limit the change/scope of this bug fix and I found more classes in this package make use of `CommonFSUtils` and `FSUtils`, I would propose and suggest to have a follow-up PR if needed. 
   
   




-- 
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

[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-852375874


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 44s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 49s |  master passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   5m  9s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 50s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/5/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/88 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 168872e2fccf 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 33d4c31 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/5/testReport/ |
   | Max. process+thread count | 957 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/5/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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

[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-848431044


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 53s |  master passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   4m 44s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/2/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/88 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 9c82051cd414 5.4.0-1043-aws #45~18.04.1-Ubuntu SMP Fri Apr 9 23:32:25 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 33d4c31 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/2/testReport/ |
   | Max. process+thread count | 970 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/2/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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

[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #88: HBASE-25921 Fix Wrong FileSystem when running `filesystem` on non-HDFS storage

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #88:
URL: https://github.com/apache/hbase-operator-tools/pull/88#issuecomment-849305692


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 18s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 4 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  0s |  master passed  |
   | +1 :green_heart: |  compile  |   0m  8s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   5m 12s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  5s |  The patch does not generate ASF License warnings.  |
   |  |   |   8m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/3/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/88 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux dd2ba8201586 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 33d4c31 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/3/testReport/ |
   | Max. process+thread count | 941 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-88/3/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
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