Skip to content

Conversation

@dnskr
Copy link

@dnskr dnskr commented Jan 18, 2022

What changes were proposed in this pull request?

The memory leak of ExecutionListenerBus was fixed in 4d90c5d by automatically cleaning it up when the SparkSession is GC'ed.
This is a similar fix for the memory leak of SQLAppStatusListener that occurs when a new SessionState is touched. The memory leak is automatically cleaned up when the SharedState is GC'ed.

Why are the changes needed?

The memory leak causes OOM in spark applications like Spark Thrift Server that creates many sessions.

Does this PR introduce any user-facing change?

No

How was this patch tested?

A test has been added to the PR

@cloud-fan
Copy link
Contributor

@dnskr Can you describe the case that multiple SharedStates are instantiated? Ideally we should only have one SharedState instance per driver JVM.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@dnskr
Copy link
Author

dnskr commented Jan 19, 2022

@cloud-fan Thanks for the question.
I checked memory dumps and I didn't find that multiple SharedStates are instantiated in the Spark Thrift Server.
It are only one instance of SharedState and only one instance of SQLAppStatusListener.
So I can conclude that this is not a reason of memory leak in my case.

However unit tests in the PR and jira ticket demonstrate potential memory leak, so:

  • Should we fix it even though it doesn't happen in real scenario (there is always only one SharedState instance per driver JVM)?
  • Should we just close the original jira ticket with proper status/comment to avoid misunderstanding in the future? I'm asking because I found the ticket by searching "leak" in jira to find why I have the memory leak issue.

@cloud-fan
Copy link
Contributor

let's close the JIRA ticket then.

@dnskr dnskr closed this Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants