[SPARK-5363] [PySpark] check ending mark in non-block way #4601

davies · 2015-02-13T23:40:51Z

There is chance of dead lock that the Python process is waiting for ending mark from JVM, but which is eaten by corrupted stream.

This PR checks the ending mark from Python in non-block way, so it will not blocked by Python process.

There is a small chance that the ending mark is sent by Python process but not available right now, then Python process will not be used.

cc @JoshRosen @pwendell

SparkQA · 2015-02-13T23:42:31Z

Test build #27463 has started for PR 4601 at commit 05e1085.

This patch merges cleanly.

SparkQA · 2015-02-14T01:00:56Z

Test build #27463 has finished for PR 4601 at commit 05e1085.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Params(

AmplabJenkins · 2015-02-14T01:01:00Z

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

JoshRosen · 2015-02-17T00:05:59Z

core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala

JoshRosen · 2015-02-17T00:07:03Z

Can we add logging for the uncommon cases here? I'd add a log message for the case where the next integer is not available and a second case for when it's not END_OF_STREAM (this log message should contain the actual integer received).

SparkQA · 2015-02-17T00:12:40Z

Test build #27598 has started for PR 4601 at commit 656d544.

This patch merges cleanly.

SparkQA · 2015-02-17T00:22:59Z

Test build #27600 has started for PR 4601 at commit 890329c.

This patch merges cleanly.

JoshRosen · 2015-02-17T00:43:46Z

core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala

The "ended cleanly" case can probably stay at logInfo (or maybe logDebug), but I think we should make this case and the other error-case into warnings so that they aren't swallowed at lower log levels.

It's a normal case, could happen when user calls take(), I think it should be INFO.

For the first one, INFO or DEBUG both work for me.

JoshRosen · 2015-02-17T00:47:09Z

Minor log-level nitpicking aside, this looks good to me.

davies · 2015-02-17T01:16:40Z

core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala

I'd like use INFO here, then we can expect one logging for each task, saying it's re-used or not.

davies · 2015-02-17T01:16:56Z

@JoshRosen updated

SparkQA · 2015-02-17T01:17:30Z

Test build #27602 has started for PR 4601 at commit e15a8c3.

This patch merges cleanly.

SparkQA · 2015-02-17T01:18:35Z

Test build #27602 has finished for PR 4601 at commit e15a8c3.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-17T01:18:36Z

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

SparkQA · 2015-02-17T01:19:01Z

Test build #27600 has finished for PR 4601 at commit 890329c.

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

AmplabJenkins · 2015-02-17T01:19:05Z

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

SparkQA · 2015-02-17T01:40:13Z

Test build #27598 has finished for PR 4601 at commit 656d544.

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

AmplabJenkins · 2015-02-17T01:40:17Z

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

JoshRosen · 2015-02-17T04:31:06Z

LGTM. I'm going to merge this into master (1.4.0), branch-1.3 (1.3.0), and branch-1.2 (1.2.2).

There is chance of dead lock that the Python process is waiting for ending mark from JVM, but which is eaten by corrupted stream. This PR checks the ending mark from Python in non-block way, so it will not blocked by Python process. There is a small chance that the ending mark is sent by Python process but not available right now, then Python process will not be used. cc JoshRosen pwendell Author: Davies Liu <[email protected]> Closes #4601 from davies/freeze and squashes the following commits: e15a8c3 [Davies Liu] update logging 890329c [Davies Liu] Merge branch 'freeze' of github.com:davies/spark into freeze 2bd2228 [Davies Liu] add more logging 656d544 [Davies Liu] Update PythonRDD.scala 05e1085 [Davies Liu] check ending mark in non-block way (cherry picked from commit ac6fe67) Signed-off-by: Josh Rosen <[email protected]>

JoshRosen · 2015-02-17T04:41:01Z

I merged this into master (1.4.0), branch-1.3 (1.3.0), and branch-1.2 (1.2.2), but did so right before I noticed that there's a comment on JIRA suggesting that this didn't fix the freeze. I guess I was a bit too trigger-happy here since I wanted to try to squeeze a fix in for 1.3.0.

Caused by #4601

davies · 2015-02-17T09:03:58Z

core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala

@JoshRosen This does not work very well in practice, it's common to see some workers can not be re-used, I will try to find a better solution, or revert this? (because it seems that it did not solve the freeze problem).

Yeah, let's revert and continue to investigate.

Great, thanks!

JoshRosen · 2015-02-17T15:52:11Z

Reverted in master (1.4.0), branch-1.3 (1.3.0), and branch-1.2 (1.2.2).

There is chance of dead lock that the Python process is waiting for ending mark from JVM, but which is eaten by corrupted stream. This PR checks the ending mark from Python in non-block way, so it will not blocked by Python process. There is a small chance that the ending mark is sent by Python process but not available right now, then Python process will not be used. cc JoshRosen pwendell Author: Davies Liu <[email protected]> Closes apache#4601 from davies/freeze and squashes the following commits: e15a8c3 [Davies Liu] update logging 890329c [Davies Liu] Merge branch 'freeze' of github.com:davies/spark into freeze 2bd2228 [Davies Liu] add more logging 656d544 [Davies Liu] Update PythonRDD.scala 05e1085 [Davies Liu] check ending mark in non-block way (cherry picked from commit ac6fe67) Signed-off-by: Josh Rosen <[email protected]>

Caused by apache#4601

check ending mark in non-block way

05e1085

JoshRosen reviewed Feb 17, 2015
View reviewed changes

core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala Outdated

Copy link

Contributor

JoshRosen Feb 17, 2015

Choose a reason for hiding this comment

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

Typo: "Tt"

Update PythonRDD.scala

656d544

Davies Liu added 2 commits February 16, 2015 16:17

add more logging

2bd2228

Merge branch 'freeze' of github.com:davies/spark into freeze

890329c

JoshRosen reviewed Feb 17, 2015
View reviewed changes

update logging

e15a8c3

davies reviewed Feb 17, 2015
View reviewed changes

asfgit closed this in ac6fe67 Feb 17, 2015

asfgit pushed a commit that referenced this pull request Feb 17, 2015

HOTFIX: Style issue causing build break

e9241fa

Caused by #4601

asfgit pushed a commit that referenced this pull request Feb 17, 2015

HOTFIX: Style issue causing build break

c06e42f

Caused by #4601

asfgit pushed a commit that referenced this pull request Feb 17, 2015

HOTFIX: Style issue causing build break

432ceca

Caused by #4601

davies reviewed Feb 17, 2015
View reviewed changes

markhamstra pushed a commit to markhamstra/spark that referenced this pull request Feb 24, 2015

HOTFIX: Style issue causing build break

3ee4394

Caused by apache#4601

[SPARK-5363] [PySpark] check ending mark in non-block way #4601

[SPARK-5363] [PySpark] check ending mark in non-block way #4601

Uh oh!

Conversation

davies commented Feb 13, 2015

Uh oh!

SparkQA commented Feb 13, 2015

Uh oh!

SparkQA commented Feb 14, 2015

Uh oh!

AmplabJenkins commented Feb 14, 2015

Uh oh!

JoshRosen Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

JoshRosen Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

davies Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Feb 17, 2015

Uh oh!

davies Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

davies commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

AmplabJenkins commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

AmplabJenkins commented Feb 17, 2015

Uh oh!

SparkQA commented Feb 17, 2015

Uh oh!

AmplabJenkins commented Feb 17, 2015

Uh oh!

JoshRosen commented Feb 17, 2015

Uh oh!

JoshRosen commented Feb 17, 2015

Uh oh!

davies Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

JoshRosen Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

davies Feb 17, 2015

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Feb 17, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants