Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HBASE-23379: Clean Up FSUtil getRegionLocalityMappingFromFS #911

Merged
merged 2 commits into from Dec 10, 2019
Merged

HBASE-23379: Clean Up FSUtil getRegionLocalityMappingFromFS #911

merged 2 commits into from Dec 10, 2019

Conversation

belugabehr
Copy link
Contributor

No description provided.

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 34s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 5m 27s master passed
+1 💚 compile 0m 54s master passed
+1 💚 checkstyle 1m 21s master passed
+1 💚 shadedjars 4m 35s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 37s master passed
+0 🆗 spotbugs 4m 11s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 4m 10s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 5m 0s the patch passed
+1 💚 compile 0m 54s the patch passed
+1 💚 javac 0m 54s the patch passed
-1 ❌ checkstyle 1m 20s hbase-server: The patch generated 1 new + 74 unchanged - 0 fixed = 75 total (was 74)
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 shadedjars 4m 35s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 15m 42s Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚 javadoc 0m 36s the patch passed
+1 💚 findbugs 4m 15s the patch passed
_ Other Tests _
+1 💚 unit 145m 51s hbase-server in the patch passed.
+1 💚 asflicense 0m 35s The patch does not generate ASF License warnings.
202m 37s
Subsystem Report/Notes
Docker Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/1/artifact/out/Dockerfile
GITHUB PR #911
JIRA Issue HBASE-23379
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname Linux 5138d2b847f6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-911/out/precommit/personality/provided.sh
git revision master / 9c82a65
Default Java 1.8.0_181
checkstyle https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/1/artifact/out/diff-checkstyle-hbase-server.txt
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/1/testReport/
Max. process+thread count 4148 (vs. ulimit of 10000)
modules C: hbase-server U: hbase-server
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/1/console
versions git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

HorizonNet
HorizonNet requested changes Dec 8, 2019
View reviewed changes
Copy link
Contributor

@HorizonNet HorizonNet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also look at the Checkstyle warning.

hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Outdated
}

// lower the number of threads in case we have very few expected regions
threadPoolSize = Math.min(threadPoolSize, statusList.length);
final int finalThreadPoolSize = Math.min(threadPoolSize, statusList.length);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave the name as threadPoolSize.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Will make requested change. Please note that it's easier to debug when the arguments can be assigned 'final' and not subject to change during the lifespan of the method call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, haven't seen this one. Agree on the final topic. Can we revert this one to final int? A better name would be good.

@belugabehr
Copy link
Contributor Author

@HorizonNet Thanks for the review!!

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 33s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚 mvninstall 5m 26s master passed
+1 💚 compile 0m 55s master passed
+1 💚 checkstyle 1m 18s master passed
+1 💚 shadedjars 4m 32s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 39s master passed
+0 🆗 spotbugs 4m 1s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 3m 58s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 4m 58s the patch passed
+1 💚 compile 0m 55s the patch passed
+1 💚 javac 0m 55s the patch passed
+1 💚 checkstyle 1m 21s the patch passed
+1 💚 whitespace 0m 1s The patch has no whitespace issues.
+1 💚 shadedjars 4m 39s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 16m 15s Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚 javadoc 0m 36s the patch passed
+1 💚 findbugs 4m 20s the patch passed
_ Other Tests _
+1 💚 unit 154m 7s hbase-server in the patch passed.
+1 💚 asflicense 0m 36s The patch does not generate ASF License warnings.
212m 27s
Subsystem Report/Notes
Docker Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/2/artifact/out/Dockerfile
GITHUB PR #911
JIRA Issue HBASE-23379
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname Linux c7642b41e5cf 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-911/out/precommit/personality/provided.sh
git revision master / 82e155e
Default Java 1.8.0_181
Test Results https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/2/testReport/
Max. process+thread count 4061 (vs. ulimit of 10000)
modules C: hbase-server U: hbase-server
Console output https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-911/2/console
versions git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

@belugabehr
Copy link
Contributor Author

Any other thoughts @HorizonNet ? Thanks!

@HorizonNet HorizonNet merged commit c39339c into apache:master Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HorizonNet HorizonNet HorizonNet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@belugabehr @Apache-HBase @HorizonNet