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-10548] [SPARK-10563] [SQL] Fix concurrent SQL executions / branch-1.5 #8721

Closed

Conversation

andrewor14
Copy link
Contributor

Note: this is for branch-1.5 only

This is the same as #8710 but affects only SQL. The more general fix for SPARK-10563 is considered risky to backport into a maintenance release, so it is disabled by default and enabled only in SQL.

@andrewor14
Copy link
Contributor Author

@zsxwing please look at this one too

@SparkQA
Copy link

SparkQA commented Sep 11, 2015

Test build #42344 has finished for PR 8721 at commit a597804.

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

@andrewor14
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Sep 11, 2015

Test build #42356 has finished for PR 8721 at commit a597804.

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

@SparkQA
Copy link

SparkQA commented Sep 12, 2015

Test build #42362 timed out for PR 8721 at commit d2a76b0 after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Sep 13, 2015

Test build #42380 has finished for PR 8721 at commit 47696b4.

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

SPARK-10548 is caused by the fact that mutating local properties
in the parent thread is reflected in the children threads. Instead,
we should just make a clone of the parent properties.
@andrewor14 andrewor14 force-pushed the concurrent-sql-executions-1.5 branch 2 times, most recently from 2be65b0 to 093f5e3 Compare September 14, 2015 21:20
@andrewor14 andrewor14 changed the title [SPARK-10548] [SQL] Fix concurrent SQL executions / branch-1.5 [SPARK-10548] [SPARK-10563] [SQL] Fix concurrent SQL executions / branch-1.5 Sep 14, 2015
…sql-executions-1.5

Conflicts:
	core/src/test/scala/org/apache/spark/ThreadingSuite.scala
@SparkQA
Copy link

SparkQA commented Sep 15, 2015

Test build #42445 has finished for PR 8721 at commit 093f5e3.

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

@SparkQA
Copy link

SparkQA commented Sep 15, 2015

Test build #42446 timed out for PR 8721 at commit 4435db7 after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Sep 15, 2015

Test build #42453 has finished for PR 8721 at commit 3b9b462.

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

@SparkQA
Copy link

SparkQA commented Sep 15, 2015

Test build #1750 has finished for PR 8721 at commit 3b9b462.

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

@SparkQA
Copy link

SparkQA commented Sep 15, 2015

Test build #1749 has finished for PR 8721 at commit 3b9b462.

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

@zsxwing
Copy link
Member

zsxwing commented Sep 15, 2015

LGTM

@andrewor14
Copy link
Contributor Author

Merging into 1.5.

asfgit pushed a commit that referenced this pull request Sep 15, 2015
…nch-1.5

*Note: this is for branch-1.5 only*

This is the same as #8710 but affects only SQL. The more general fix for SPARK-10563 is considered  risky to backport into a maintenance release, so it is disabled by default and enabled only in SQL.

Author: Andrew Or <andrew@databricks.com>

Closes #8721 from andrewor14/concurrent-sql-executions-1.5 and squashes the following commits:

3b9b462 [Andrew Or] Merge branch 'branch-1.5' of github.com:apache/spark into concurrent-sql-executions-1.5
4435db7 [Andrew Or] Clone properties only for SQL for backward compatibility
0b7e5ab [Andrew Or] Clone parent local properties on inherit
asfgit pushed a commit that referenced this pull request Sep 15, 2015
*Note: this is for master branch only.* The fix for branch-1.5 is at #8721.

The query execution ID is currently passed from a thread to its children, which is not the intended behavior. This led to `IllegalArgumentException: spark.sql.execution.id is already set` when running queries in parallel, e.g.:
```
(1 to 100).par.foreach { _ =>
  sc.parallelize(1 to 5).map { i => (i, i) }.toDF("a", "b").count()
}
```
The cause is `SparkContext`'s local properties are inherited by default. This patch adds a way to exclude keys we don't want to be inherited, and makes SQL go through that code path.

Author: Andrew Or <andrew@databricks.com>

Closes #8710 from andrewor14/concurrent-sql-executions.
@andrewor14 andrewor14 closed this Sep 15, 2015
@andrewor14 andrewor14 deleted the concurrent-sql-executions-1.5 branch September 15, 2015 23:49
asfgit pushed a commit that referenced this pull request Sep 22, 2015
… job description in streaming jobs

**Note that this PR only for branch 1.5. See #8781 for the solution for Spark master.**

The job group, and job descriptions information is passed through thread local properties, and get inherited by child threads. In case of spark streaming, the streaming jobs inherit these properties from the thread that called streamingContext.start(). This may not make sense.

1. Job group: This is mainly used for cancelling a group of jobs together. It does not make sense to cancel streaming jobs like this, as the effect will be unpredictable. And its not a valid usecase any way, to cancel a streaming context, call streamingContext.stop()

2. Job description: This is used to pass on nice text descriptions for jobs to show up in the UI. The job description of the thread that calls streamingContext.start() is not useful for all the streaming jobs, as it does not make sense for all of the streaming jobs to have the same description, and the description may or may not be related to streaming.

The solution in this PR is meant for the Spark branch 1.5, where local properties are inherited by cloning the properties only when the Spark config `spark.localProperties.clone` is set to `true` (see #8781 for the PR for Spark master branch). Similar to the approach taken by #8721, StreamingContext sets that configuration to true, which makes sure that all subsequent child threads get a cloned copy of the threadlocal properties. This allows the job group and job description to be explicitly removed in the thread that starts the streaming scheduler, so that all the subsequent child threads does not inherit them. Also, the starting is done in a new child thread, so that setting the job group and description for streaming, does not change those properties in the thread that called streamingContext.start().

Author: Tathagata Das <tathagata.das1565@gmail.com>

Closes #8856 from tdas/SPARK-10649-1.5.
ashangit pushed a commit to ashangit/spark that referenced this pull request Oct 19, 2016
…nch-1.5

*Note: this is for branch-1.5 only*

This is the same as apache#8710 but affects only SQL. The more general fix for SPARK-10563 is considered  risky to backport into a maintenance release, so it is disabled by default and enabled only in SQL.

Author: Andrew Or <andrew@databricks.com>

Closes apache#8721 from andrewor14/concurrent-sql-executions-1.5 and squashes the following commits:

3b9b462 [Andrew Or] Merge branch 'branch-1.5' of github.com:apache/spark into concurrent-sql-executions-1.5
4435db7 [Andrew Or] Clone properties only for SQL for backward compatibility
0b7e5ab [Andrew Or] Clone parent local properties on inherit

(cherry picked from commit 997be78)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants