Skip to content

Conversation

@bbeaudreault
Copy link
Contributor

@bbeaudreault bbeaudreault commented Nov 30, 2023

This change reverts HBASE-12819. Please see my reasonings here: https://issues.apache.org/jira/browse/HBASE-28222?focusedCommentId=17791815&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17791815

I tried implementing a few different ways to solve this leak while leaving the above jira in place, but none of them quite work or make sense. We really need caching enabled given how much of our hdfs path code works.

I've tested this in my environment, and it fixes a longstanding leak-to-OOM for us. In the past we worked around it with a custom patch.

Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb
@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 37s 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.
_ master Compile Tests _
+1 💚 mvninstall 2m 46s master passed
+1 💚 compile 0m 33s master passed
+1 💚 checkstyle 0m 12s master passed
+1 💚 spotless 0m 42s branch has no errors when running spotless:check.
+1 💚 spotbugs 0m 35s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 42s the patch passed
+1 💚 compile 0m 30s the patch passed
+1 💚 javac 0m 30s the patch passed
+1 💚 checkstyle 0m 12s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 9m 28s Patch does not cause any errors with Hadoop 3.2.4 3.3.6.
+1 💚 spotless 0m 41s patch has no errors when running spotless:check.
+1 💚 spotbugs 0m 40s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 8s The patch does not generate ASF License warnings.
25m 36s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #5554
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname Linux 9384a4a8c923 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 44eb408
Default Java Eclipse Adoptium-11.0.17+8
Max. process+thread count 80 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/console
versions git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 34s Docker mode activated.
-0 ⚠️ yetus 0m 2s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 2m 53s master passed
+1 💚 compile 0m 19s master passed
+1 💚 shadedjars 4m 52s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 17s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 38s the patch passed
+1 💚 compile 0m 20s the patch passed
+1 💚 javac 0m 20s the patch passed
+1 💚 shadedjars 4m 50s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 15s the patch passed
_ Other Tests _
+1 💚 unit 16m 55s hbase-mapreduce in the patch passed.
34m 49s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #5554
Optional Tests javac javadoc unit shadedjars compile
uname Linux fc3c3460795f 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 44eb408
Default Java Eclipse Adoptium-11.0.17+8
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/testReport/
Max. process+thread count 3252 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 50s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 3m 57s master passed
+1 💚 compile 0m 20s master passed
+1 💚 shadedjars 5m 58s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 15s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 20s the patch passed
+1 💚 compile 0m 22s the patch passed
+1 💚 javac 0m 22s the patch passed
+1 💚 shadedjars 5m 54s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 15s the patch passed
_ Other Tests _
+1 💚 unit 17m 8s hbase-mapreduce in the patch passed.
39m 20s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #5554
Optional Tests javac javadoc unit shadedjars compile
uname Linux f497d2d8738d 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 44eb408
Default Java Temurin-1.8.0_352-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/testReport/
Max. process+thread count 3162 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5554/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@wchevreuil wchevreuil left a comment

Choose a reason for hiding this comment

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

👍
The explanation in the jira makes sense to me, I'm convinced!

@bbeaudreault bbeaudreault merged commit 02217e1 into master Dec 1, 2023
@bbeaudreault bbeaudreault deleted the HBASE-28222 branch December 1, 2023 13:23
bbeaudreault added a commit that referenced this pull request Dec 1, 2023
Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb

Signed-off-by: Wellington Chevreuil <[email protected]>
bbeaudreault added a commit that referenced this pull request Dec 1, 2023
Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb

Signed-off-by: Wellington Chevreuil <[email protected]>
bbeaudreault added a commit that referenced this pull request Dec 1, 2023
Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb

Signed-off-by: Wellington Chevreuil <[email protected]>
bbeaudreault added a commit to HubSpot/hbase that referenced this pull request Dec 1, 2023
…pshot on S3A (apache#5554)

Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb

Signed-off-by: Wellington Chevreuil <[email protected]>
kadirozde pushed a commit to kadirozde/hbase that referenced this pull request Jan 5, 2024
…he#5554)

Revert "HBASE-12819 ExportSnapshot doesn't close FileSystem instances"

This reverts commit ee32eeb

Signed-off-by: Wellington Chevreuil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants