[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
GitHub user carsonwang opened a pull request: https://github.com/apache/spark/pull/9991 [SPARK-11206] (Followup) Fix SQLListenerMemoryLeakSuite test error A followup to #9297, fix the SQLListenerMemoryLeakSuite test error. The [failure](https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-pre-YARN/HADOOP_VERSION=2.0.0-mr1-cdh4.1.2,label=spark-test/4896/) occurs because a `sqlListener` created by a previous test suite is not cleared in the SQLListenerMemoryLeakSuite. In the failure case, the previous test suite DateFunctionsSuite has 91 completed executions. So the error message is `91 was not less than or equal to 50.` For test suites extends `SharedSQLContext`, the sqlListener is cleared in the method `beforeAll`. Since `SQLListenerMemoryLeakSuite` doesn't extends `SharedSQLContext`, the `sqlListener` need to be cleared manually before creating the `SQLContext`. /cc @vanzin @JoshRosen @chenghao-intel You can merge this pull request into a Git repository by running: $ git pull https://github.com/carsonwang/spark SQLListenerTestFix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/9991.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #9991 commit 8ca3031948b56f5f54dced1b58de35ec27f1e370 Author: Carson Wang Date: 2015-11-26T06:41:10Z Fix SQLListenerMemoryLeakSuite test error --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-159825648 **[Test build #46751 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46751/consoleFull)** for PR 9991 at commit [`8ca3031`](https://github.com/apache/spark/commit/8ca3031948b56f5f54dced1b58de35ec27f1e370). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r45948582
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
This is not a public API. So the user cannot clear
`SQLContext.sqlListener`? This will be a memory leak considering `SQLListener`
usually stores a lot of data.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r45948964
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
Previously each `SQLContext` has its own `sqlListener`. Because now the SQL
events are posted to the event bus. All `SQLContext` now share a single
`sqlListener`. I don't think a user need clear `SQLContext.sqlListener`. This
is only used by the unit tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r45949221
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
See https://issues.apache.org/jira/browse/SPARK-11700
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r45949524
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
SPARK-11700 is a bit different. But my point is we should not keep a big
object in memory and don't provide an approach to clean it. In some user cases,
Spark SQL may be just one of some ETL steps. And if the user finishes his/her
work in Spark SQL, he/she usually wants to clean up all resources used by
SparkContext/SQLContext.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r45952482
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
I see. Is it enough to make `SQLContext.clearSqlListner` public here? So we
provide a way to clear the reference for users who want the object to be GCed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-159848673 **[Test build #46751 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46751/consoleFull)** for PR 9991 at commit [`8ca3031`](https://github.com/apache/spark/commit/8ca3031948b56f5f54dced1b58de35ec27f1e370). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-159848849 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46751/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-159848848 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user steveloughran commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46008320
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
.. I can imagine Zeppelin wanting to purge these, or whatever Spark Kernel
is named as.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46014967
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
I think we can add a SparkContext stop hook. When SparkContext is being
stopped, clear the reference. The user doesn't have to call a method to clear
the sqlListener reference. The sqlListener is added to SparkContext and will
only be garbage collected when SparkContext is stopped.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160053650 **[Test build #46805 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46805/consoleFull)** for PR 9991 at commit [`4549f62`](https://github.com/apache/spark/commit/4549f62866f8917451dcc4d775943b5232186c46). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160055041 The original purpose of this PR is to fix the `SQLListenerMemoryLeakSuite` test failure. This can be resolved by clearing `SQLContext.sqlListener` before the test. To prevent memory leak similar to SPARK-11700, I added a `SparkContext` stop hook to clear the `sqlListener` reference. I didn't make `SQLContext.clearSqlListener` a public API because it seems a little confusing for users to call it. And the `sqlListener` is added to SparkContext, it will not be GCed at once even if a user calls `SQLContext.clearSqlListener`. Now we clear the reference when the `SparkContext` is being stopped to allow the `sqlListener` to be GCed later. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160057079 **[Test build #46807 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46807/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160112715 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160112569 **[Test build #46805 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46805/consoleFull)** for PR 9991 at commit [`4549f62`](https://github.com/apache/spark/commit/4549f62866f8917451dcc4d775943b5232186c46). * This patch **fails from timeout after a configured wait of \`250m\`**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160112718 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46805/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160118554 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46807/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160118520 **[Test build #46807 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46807/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). * This patch **fails from timeout after a configured wait of \`250m\`**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160118552 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160195567 > Test build #46807 has finished for PR 9991 at commit b694e27. > > This patch fails from timeout after a configured wait of `250m`. > This patch merges cleanly. > This patch adds no public classes. This one should be fixed in #10011 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160195573 retest this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160196391 **[Test build #46828 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46828/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160204175 **[Test build #46828 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46828/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160204204 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160204205 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46828/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160242651 retest this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160242817 **[Test build #46834 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46834/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160250373 **[Test build #46834 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46834/consoleFull)** for PR 9991 at commit [`b694e27`](https://github.com/apache/spark/commit/b694e27979a2acad3e9653e082c08e8b3f7e41b7). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160250394 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46834/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160250393 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46086313
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -1764,6 +1773,7 @@ class SparkContext(config: SparkConf) extends Logging
with ExecutorAllocationCli
// Unset YARN mode system env variable, to allow switching between
cluster types.
System.clearProperty("SPARK_YARN_MODE")
SparkContext.clearActiveContext()
+_stopHooks.foreach(_())
--- End diff --
I'm not a big fan of adding more hooks, but since SQLContext doesn't have a
`stop` method... anyway, probably good to wrap calls to the hooks with
`Utils.tryLogNonFatalError`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160352795 Would things work if `SQLListener` cleaned up after itself when `SparkListenerApplicationEnd` is received? That would avoid adding hooks in `SparkContext`. Also, does the history server suffer from this problem now that it can instantiate `SQLListener` via `SparkHistoryListenerFactory`? If it does, handling `SparkListenerApplicationEnd` would probably solve that problem too. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160371284 Haven't followed discussion in detail yet, but just wanted to flag this PR/discussion as a high priority item to get resolved soon, since the failing memory leak test is preventing the Maven builds from running certain subsequent suites. We should try to get this fixed before we start merging a bunch of patches on Monday. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160493581 **[Test build #46862 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46862/consoleFull)** for PR 9991 at commit [`00df329`](https://github.com/apache/spark/commit/00df329c8af56949e2e38a3a8f0218a9d6cc6c9e). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160494777 @vanzin , I wrapped the calls to the hooks with `Utils.tryLogNonFatalError`. I didn't clean up the `SQLListener` after a application end event because another `SQLContext` created later still wants to use the same `SQLListener`. I think the history server does not have the problem because there is no reference to the `SQLListener` from a companion object in that case. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160495888 @zsxwing , do you have any further comments regarding how the `SQLListener` is cleaned up? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160512207 **[Test build #46862 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46862/consoleFull)** for PR 9991 at commit [`00df329`](https://github.com/apache/spark/commit/00df329c8af56949e2e38a3a8f0218a9d6cc6c9e). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160512249 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160512250 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46862/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160691520 > I didn't clean up the SQLListener after a application end event because another SQLContext created later still wants to use the same SQLListener. Sorry, I don't follow. `SparkListenerApplicationEnd` is posted by `SparkContext.stop`, which is the same place where you're adding the hook to clean up the listener. So it should behave exactly the same way, no? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user zsxwing commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160714495 > Sorry, I don't follow. SparkListenerApplicationEnd is posted by SparkContext.stop, which is the same place where you're adding the hook to clean up the listener. So it should behave exactly the same way, no? It's a bit different because the location of `postApplicationEnd()` is at the beginning of `stop`? Not related to this issue: I just noticed the location in `postApplicationEnd()` may be not in the correct place. There is a race condition that after `postApplicationEnd()`, some task/job events will still be put into the listener bus. Is it safe to move `postApplicationEnd()` to the bottom of `stop`? Then I think it's exactly the same way that adding a hook. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160717545 > It's a bit different because the location of postApplicationEnd() is at the beginning of stop? It's a bit different but not in the way @carsonwang explained; whether you use the hook or handle `SparkListenerApplicationEnd`, the listener will be cleared when the context is shut down. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46206482
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -1619,6 +1620,14 @@ class SparkContext(config: SparkConf) extends
Logging with ExecutorAllocationCli
}
/**
+ * Adds a stop hook which can be used to clean up additional resource.
This is called when the
+ * sparkContext is being stopped.
+ */
+ private[spark] def addStopHook(hook: () => Unit): Unit = {
--- End diff --
Does this method (or the collection that it manipulates) need to be
synchronized in order to guard against race conditions?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46207010
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
[Over on the original PR, I
commented](https://github.com/apache/spark/pull/9297/files#r46203496) to ask
why `SQLContext.sqlListener` needs to be an `AtomicReference[SQLListener]`
instead of an `AtomicBoolean` or some other sort of atomic primitive. As far as
I can tell, we never access any methods or fields of the `sqlListener` that's
stored here, so if we only need to set something for compare-and-swap purposes
then I think we shouldn't use an `AtomicReference`, thereby avoiding the GC
issues that it causes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160770491 Head's up: since discussion here is still ongoing and I think there's still more work to do, I'm going to revert #9297 in order to un-break the master Maven tests. Could you re-submit a new PR which contains both the code from the original PR and which addresses the state lifecycle issues being discussed here? Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160858339 > It's a bit different but not in the way @carsonwang explained; whether you use the hook or handle SparkListenerApplicationEnd, the listener will be cleared when the context is shut down. Sorry I misunderstood it. Ok I will use SparkListenerApplicationEnd. The issue @zsxwing mentioned can probably be addressed in another PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/9991#discussion_r46245206
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala
---
@@ -343,6 +343,8 @@ class SQLListenerMemoryLeakSuite extends SparkFunSuite {
.set("spark.sql.ui.retainedExecutions", "50") // Set it to 50 to run
this test quickly
val sc = new SparkContext(conf)
try {
+ // Clear the sql listener created by a previous test suite.
+ SQLContext.clearSqlListener()
--- End diff --
Many unit tests use `sqlContext.listener`. Can you please suggest how to
update the unit tests if we changed to use an `AtomicBoolean`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang commented on the pull request: https://github.com/apache/spark/pull/9991#issuecomment-160876864 Close this and resubmit #10061 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-11206] (Followup) Fix SQLListenerMemory...
Github user carsonwang closed the pull request at: https://github.com/apache/spark/pull/9991 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
