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-22471][SQL] SQLListener consumes much memory causing OutOfMemoryError #19711

Closed
wants to merge 7 commits into from
Closed

[SPARK-22471][SQL] SQLListener consumes much memory causing OutOfMemoryError #19711

wants to merge 7 commits into from

Conversation

tashoyan
Copy link
Contributor

@tashoyan tashoyan commented Nov 9, 2017

What changes were proposed in this pull request?

This PR addresses the issue SPARK-22471. The modified version of SQLListener respects the setting spark.ui.retainedStages and keeps the number of the tracked stages within the specified limit. The hash map _stageIdToStageMetrics does not outgrow the limit, hence overall memory consumption does not grow with time anymore.

A 2.2-compatible fix. Maybe incompatible with 2.3 due to #19681.

How was this patch tested?

A new unit test covers this fix - see SQLListenerMemorySuite.scala.

@vanzin
Copy link
Contributor

vanzin commented Nov 9, 2017

ok to test

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM.

@@ -207,6 +210,14 @@ class SQLListener(conf: SparkConf) extends SparkListener with Logging {
}
}

override def onStageCompleted(stageCompleted: SparkListenerStageCompleted): Unit = {
Copy link
Contributor

Choose a reason for hiding this comment

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

synchronized

@SparkQA
Copy link

SparkQA commented Nov 10, 2017

Test build #83656 has finished for PR 19711 at commit 4a0582d.

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

@@ -113,7 +116,7 @@ class SQLListener(conf: SparkConf) extends SparkListener with Logging {
*/
private val _jobIdToExecutionId = mutable.HashMap[Long, Long]()

private val _stageIdToStageMetrics = mutable.HashMap[Long, SQLStageMetrics]()
private val _stageIdToStageMetrics = mutable.LinkedHashMap[Long, SQLStageMetrics]()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use Java's LinkedHashMap and override removeEldestEntry to what we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

removeEldestEntry is a protected method.

Copy link
Contributor

@jerryshao jerryshao Nov 14, 2017

Choose a reason for hiding this comment

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

Java's LinkedHashMap can be overridden with a custom implementation of removeEldestEntry, that will save the codes done below. It is not the user who call this removeEldestEntry...

@SparkQA
Copy link

SparkQA commented Nov 10, 2017

Test build #83680 has finished for PR 19711 at commit 3ecce56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Nov 10, 2017

Test build #83682 has finished for PR 19711 at commit 3ecce56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tashoyan
Copy link
Contributor Author

Corrupted build node?

@tashoyan
Copy link
Contributor Author

Retest this please.

1 similar comment
@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Nov 13, 2017

Test build #83805 has finished for PR 19711 at commit 3ecce56.

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

@vanzin
Copy link
Contributor

vanzin commented Nov 13, 2017

LGTM, merging to 2.2.

asfgit pushed a commit that referenced this pull request Nov 13, 2017
…ryError

## What changes were proposed in this pull request?

This PR addresses the issue [SPARK-22471](https://issues.apache.org/jira/browse/SPARK-22471). The modified version of `SQLListener` respects the setting `spark.ui.retainedStages` and keeps the number of the tracked stages within the specified limit. The hash map `_stageIdToStageMetrics` does not outgrow the limit, hence overall memory consumption does not grow with time anymore.

A 2.2-compatible fix. Maybe incompatible with 2.3 due to #19681.

## How was this patch tested?

A new unit test covers this fix - see `SQLListenerMemorySuite.scala`.

Author: Arseniy Tashoyan <tashoyan@gmail.com>

Closes #19711 from tashoyan/SPARK-22471-branch-2.2.
@vanzin
Copy link
Contributor

vanzin commented Nov 13, 2017

@tashoyan please close this since github doesn't do it automatically for branches.

@tashoyan tashoyan closed this Nov 14, 2017
@tashoyan tashoyan deleted the SPARK-22471-branch-2.2 branch December 15, 2017 07:16
MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
…ryError

## What changes were proposed in this pull request?

This PR addresses the issue [SPARK-22471](https://issues.apache.org/jira/browse/SPARK-22471). The modified version of `SQLListener` respects the setting `spark.ui.retainedStages` and keeps the number of the tracked stages within the specified limit. The hash map `_stageIdToStageMetrics` does not outgrow the limit, hence overall memory consumption does not grow with time anymore.

A 2.2-compatible fix. Maybe incompatible with 2.3 due to apache#19681.

## How was this patch tested?

A new unit test covers this fix - see `SQLListenerMemorySuite.scala`.

Author: Arseniy Tashoyan <tashoyan@gmail.com>

Closes apache#19711 from tashoyan/SPARK-22471-branch-2.2.
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.

5 participants