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.

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

[SPARK-12961][Core] Prevent snappy-java memory leak #10875

Closed
wants to merge 1 commit into from

Conversation

viirya
Copy link
Member

@viirya viirya commented Jan 22, 2016

JIRA: https://issues.apache.org/jira/browse/SPARK-12961

To prevent memory leak in snappy-java, just call the method once and cache the result. After the library releases new version, we can remove this object.

@JoshRosen

@SparkQA
Copy link

SparkQA commented Jan 22, 2016

Test build #49901 has finished for PR 10875 at commit d206291.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Jan 23, 2016

Seems fine

@@ -149,12 +149,7 @@ class LZFCompressionCodec(conf: SparkConf) extends CompressionCodec {
*/
@DeveloperApi
class SnappyCompressionCodec(conf: SparkConf) extends CompressionCodec {

try {
Snappy.getNativeLibraryVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

This was added by @ksakellis in #3119; the original motivation related to error reporting. I suppose there's a chance that the change in this patch changes that behavior slightly, but that might not be a huge deal. Let's see.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I guess getNativeLibraryVersion might be kind of expensive due to the IO that it does, so maybe calling it less frequently will give us a perf. boost?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think so.

@viirya
Copy link
Member Author

viirya commented Jan 26, 2016

@JoshRosen @srowen Is this ready to merge?

@srowen
Copy link
Member

srowen commented Jan 26, 2016

Merged to master/1.6

asfgit pushed a commit that referenced this pull request Jan 26, 2016
JIRA: https://issues.apache.org/jira/browse/SPARK-12961

To prevent memory leak in snappy-java, just call the method once and cache the result. After the library releases new version, we can remove this object.

JoshRosen

Author: Liang-Chi Hsieh <viirya@gmail.com>

Closes #10875 from viirya/prevent-snappy-memory-leak.

(cherry picked from commit 5936bf9)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@asfgit asfgit closed this in 5936bf9 Jan 26, 2016
@viirya viirya deleted the prevent-snappy-memory-leak branch December 27, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants