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.

Sign up for GitHub

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

Jump to bottom

[SQL] Fixes race condition in CliSuite #3060

Closed
wants to merge 1 commit into from
Closed

[SQL] Fixes race condition in CliSuite #3060

wants to merge 1 commit into from
+15 −20

Conversation

liancheng
Copy link
Contributor

CliSuite has been flaky for a while, this PR tries to improve this situation by fixing a race condition in CliSuite. The captureOutput function is used to capture both stdout and stderr output of the forked external process in two background threads and search for expected strings, but wasn't been properly synchronized before.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22760 has started for PR 3060 at commit a70569c.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22760 has finished for PR 3060 at commit a70569c.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22760/
Test PASSed.

@liancheng liancheng mentioned this pull request Nov 2, 2014
Closed
1 task
@liancheng
Copy link
Contributor Author

retest this please

(The fixed issue is a race condition, would like to run multiple Jenkins builds to verify.)

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #505 has started for PR 3060 at commit a70569c.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #505 has finished for PR 3060 at commit a70569c.

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

@scwf
Copy link
Contributor

scwf commented Nov 2, 2014

aha, it explains the timeout issue. is there the same problem in HiveThriftServer2Suite?

@scwf
Copy link
Contributor

scwf commented Nov 2, 2014

retest this please.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22762 has started for PR 3060 at commit a70569c.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22762 has finished for PR 3060 at commit a70569c.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22762/
Test PASSed.

@marmbrus
Copy link
Contributor

marmbrus commented Nov 2, 2014

Thanks for figuring this out! Merged to master.

@asfgit asfgit closed this in 495a132 Nov 2, 2014
@liancheng liancheng deleted the fix-cli-suite branch November 3, 2014 01:49
@liancheng liancheng mentioned this pull request Nov 5, 2014
Closed
liancheng added a commit to liancheng/spark that referenced this pull request Nov 9, 2014
@liancheng
[SQL] Fixes race condition in CliSuite
7f395b7 
`CliSuite` has been flaky for a while, this PR tries to improve this situation by fixing a race condition in `CliSuite`. The `captureOutput` function is used to capture both stdout and stderr output of the forked external process in two background threads and search for expected strings, but wasn't been properly synchronized before.

Author: Cheng Lian <lian@databricks.com>

Closes apache#3060 from liancheng/fix-cli-suite and squashes the following commits:

a70569c [Cheng Lian] Fixes race condition in CliSuite
marmbrus pushed a commit to marmbrus/spark that referenced this pull request Nov 11, 2014
@liancheng @marmbrus
[SPARK-3971][SQL] Backport apache#2843 to branch-1.1
64945f8 
This PR backports apache#2843 to branch-1.1. The key difference is that this one doesn't support Hive 0.13.1 and thus always returns `0.12.0` when `spark.sql.hive.version` is queried.

6 other commits on which apache#2843 depends were also backported, they are:

- apache#2887 for `SessionState` lifecycle control
- apache#2675, apache#2823 & apache#3060 for major test suite refactoring and bug fixes
- apache#2164, for Parquet test suites updates
- apache#2493, for reading `spark.sql.*` configurations

Author: Cheng Lian <lian@databricks.com>
Author: Cheng Lian <lian.cs.zju@gmail.com>
Author: Michael Armbrust <michael@databricks.com>

Closes apache#3113 from liancheng/get-info-for-1.1 and squashes the following commits:

d354161 [Cheng Lian] Provides Spark and Hive version in HiveThriftServer2 for branch-1.1
0c2a244 [Michael Armbrust] [SPARK-3646][SQL] Copy SQL configuration from SparkConf when a SQLContext is created.
3202a36 [Michael Armbrust] [SQL] Decrease partitions when testing
7f395b7 [Cheng Lian] [SQL] Fixes race condition in CliSuite
0dd28ec [Cheng Lian] [SQL] Fixes the race condition that may cause test failure
5928b39 [Cheng Lian] [SPARK-3809][SQL] Fixes test suites in hive-thriftserver
faeca62 [Cheng Lian] [SPARK-4037][SQL] Removes the SessionState instance created in HiveThriftServer2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@liancheng @SparkQA @AmplabJenkins @scwf @marmbrus