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/01/13 17:32:02 UTC
[GitHub] [hbase] joshelser opened a new pull request #1029: HBASE-23679
FileSystem objects leak when cleaned up in cleanupBulkLoad
joshelser opened a new pull request #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029
The cleanupBulkLoad method is only called for the first Region in the
table which was being bulk loaded into. This means that potentially N-1
other RegionServers (where N is the number of RegionServers) will leak
one FileSystem object into the FileSystem cache which will never be
cleaned up. We need to do this clean-up as a part of secureBulkLoadHFiles
otherwise we cannot guarantee that heap usage won't grow unbounded.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1029: HBASE-23679
FileSystem objects leak when cleaned up in cleanupBulkLoad
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029#issuecomment-573871716
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | 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 :green_heart: | mvninstall | 5m 27s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 19s | master passed |
| +1 :green_heart: | shadedjars | 4m 34s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
| +0 :ok: | spotbugs | 4m 24s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 56s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 16s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 39s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
| +1 :green_heart: | findbugs | 4m 27s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 157m 11s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
| | | 214m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1029/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1029 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b150b6101edd 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1029/out/precommit/personality/provided.sh |
| git revision | master / 0bf933b068 |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1029/1/testReport/ |
| Max. process+thread count | 5571 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1029/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.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] asfgit closed pull request #1029: HBASE-23679 FileSystem
objects leak when cleaned up in cleanupBulkLoad
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on issue #1029: HBASE-23679 FileSystem
objects leak when cleaned up in cleanupBulkLoad
Posted by GitBox <gi...@apache.org>.
joshelser commented on issue #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029#issuecomment-573781272
The problem with #1019 was that other code called beneath SecureBulkLoadManager would expect that there's a cached FileSystem object for it to use. While this is good for us to go through and not willy-nilly expect there to be a cached FS instance hanging around, it does complicate a commit which was intending to simplify things.
Thus, I think it's better for us to go the easy-route, fixing SecureBulkLoadManager.
I've been testing this by letting two instances of IntegrationTestBulkLoad run concurrently with a small chainLength (to let more bulk loads happen), looking at the output of `jmap histo:live` to see how many `DistributedFileSystem` instances we have strongly referenced (it should never be higher than the 3 FS instances we have open, plus the number of concurrent bulk loads, 2, we could have running). It's been running for about an hour; I'll let it continue until QA comes back, or until my HDFS fills up.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] joshelser commented on issue #1029: HBASE-23679 FileSystem
objects leak when cleaned up in cleanupBulkLoad
Posted by GitBox <gi...@apache.org>.
joshelser commented on issue #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029#issuecomment-573831673
Thanks for the quick reviews, folks.
> Maybe as future work, if it is indeed worth due to potential performance impacts (i.e. case of retries), could LoadIncrementalHFiles call the cleanup for each region involved in the bulkload, once it's done?
Yeah, admittedly, I don't have a good understanding for what the performance impact is. We would have to do some new SASL handshakes to HDFS daemons, but, beyond that, I'm not sure what the actual cost is. I also did not look historically to see if this code once made sense :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] busbey commented on a change in pull request #1029:
HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #1029: HBASE-23679 FileSystem objects leak when cleaned up in cleanupBulkLoad
URL: https://github.com/apache/hbase/pull/1029#discussion_r365960514
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java
##########
@@ -305,6 +301,13 @@ private boolean isUserReferenced(UserGroupInformation ugi) {
});
} finally {
decrementUgiReference(ugi);
+ try {
+ if (!UserGroupInformation.getLoginUser().equals(ugi) && !isUserReferenced(ugi)) {
+ FileSystem.closeAllForUGI(ugi);
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to close FileSystem for: " + ugi, e);
Review comment:
nit: parameterize the log message.
----------------------------------------------------------------
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
With regards,
Apache Git Services