[SPARK-4109][CORE] Correctly deserialize Task.stageId by luluorta · Pull Request #2971 · apache/spark

luluorta · 2014-10-28T05:50:08Z

The two subclasses of Task, ShuffleMapTask and ResultTask, do not correctly deserialize stageId. Therefore, the accessing of TaskContext.stageId always returns zero value to the user.

AmplabJenkins · 2014-10-28T05:52:10Z

Can one of the admins verify this patch?

srowen · 2014-10-28T18:10:06Z

Question, does this need to be renamed? it's just a constructor parameter. Later the assignment to stageId is already setting the parent value?

you don't actually need to rename this. scala will handle it.

renaming is better, otherwise simply by accessing stageId you could upgrade it to a field accidentally

Scala is smart enough to not to do that actually.

Actually hold on - I tested it again and the compiler is not doing the right thing. Let me verify.

Take a look at here: https://gist.github.com/rxin/6be132f46b72c27d8f89
I think if the parent constructor declares the field as val or var, then it is fine. Otherwise it is not. So technically in this case it should be fine to not rename, but it is better to rename it explicitly just to avoid the compiler behavior change (since this might be a corner case) in the future.

andrewor14 · 2014-10-28T19:50:12Z

add to whitelist

SparkQA · 2014-10-28T19:54:56Z

Test build #22370 has started for PR 2971 at commit ff35ee6.

This patch merges cleanly.

SparkQA · 2014-10-28T21:10:31Z

Test build #22370 has finished for PR 2971 at commit ff35ee6.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-28T21:10:35Z

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

JoshRosen · 2014-10-29T06:13:11Z

As long as you're modifying this code, mind tossing a Utils.tryOrIOException here so that any errors that occur here are reported properly? See #2932 for explanation / context.

I guess I missed this case in my PR.

rxin · 2014-11-01T08:26:30Z

Jenkins, retest this please.

SparkQA · 2014-11-01T08:27:40Z

Test build #22689 has started for PR 2971 at commit ff35ee6.

This patch merges cleanly.

SparkQA · 2014-11-01T09:42:12Z

Test build #22689 has finished for PR 2971 at commit ff35ee6.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-01T09:42:16Z

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

rxin · 2014-11-01T21:44:07Z

Jenkins, retest this please.

SparkQA · 2014-11-01T21:50:05Z

Test build #22702 has started for PR 2971 at commit ff35ee6.

This patch merges cleanly.

SparkQA · 2014-11-01T23:06:26Z

Test build #22702 has finished for PR 2971 at commit ff35ee6.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-01T23:06:30Z

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

rxin · 2014-11-02T04:36:36Z

Jenkins, retest this please.

SparkQA · 2014-11-02T04:39:54Z

Test build #22740 has started for PR 2971 at commit ff35ee6.

This patch merges cleanly.

SparkQA · 2014-11-02T04:48:29Z

Test build #22740 has finished for PR 2971 at commit ff35ee6.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-02T04:48:31Z

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

rxin · 2014-11-02T06:47:31Z

Jenkins, retest this please.

SparkQA · 2014-11-02T06:54:54Z

Test build #22751 has started for PR 2971 at commit ff35ee6.

This patch merges cleanly.

SparkQA · 2014-11-02T09:08:32Z

Test build #22751 has finished for PR 2971 at commit ff35ee6.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-02T09:08:37Z

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

rxin · 2014-11-02T20:09:55Z

cc @marmbrus @liancheng any idea why the unit test for Python is failing? Are there some unit tests testing the stageId?

marmbrus · 2014-11-02T22:18:10Z

Hmmm, not sure what is going on here, especially since other SQL PRs seem to be passing at the moment. @davies any idea?

davies · 2014-11-03T02:25:51Z

@marmbrus This PR is targeted for 1.0, but PySpark SQL 1.0 can not passed the tests in Python 2.6 (Pyrolite can not deserialize of array.array from Python 2.6).

We should change the default Python to 2.7 for testing spark 1.0 branch.

It's not related to this PR.

rxin · 2014-11-03T05:52:23Z

Ok I'm going to merge this and patch it for master as well.

The two subclasses of Task, ShuffleMapTask and ResultTask, do not correctly deserialize stageId. Therefore, the accessing of TaskContext.stageId always returns zero value to the user. Author: luluorta <luluorta@gmail.com> Closes#2971 from luluorta/fix-task-ser and squashes the following commits: ff35ee6 [luluorta] correctly deserialize Task.stageId

correctly deserialize Task.stageId
ff35ee6

luluorta changed the title ~~[SPARK-4109] Correctly deserialize Task.stageId~~[SPARK-4109][CORE] Correctly deserialize Task.stageIdOct 28, 2014

srowen reviewed Oct 28, 2014
View reviewed changes

JoshRosen reviewed Oct 29, 2014
View reviewed changes

Uh oh!

Conversation

luluorta commented Oct 28, 2014

Uh oh!

AmplabJenkins commented Oct 28, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 28, 2014

Uh oh!

AmplabJenkins commented Oct 28, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxin commented Nov 1, 2014

Uh oh!

SparkQA commented Nov 1, 2014

Uh oh!

SparkQA commented Nov 1, 2014

Uh oh!

AmplabJenkins commented Nov 1, 2014

Uh oh!

rxin commented Nov 1, 2014

Uh oh!

SparkQA commented Nov 1, 2014

Uh oh!

SparkQA commented Nov 1, 2014

Uh oh!

AmplabJenkins commented Nov 1, 2014

Uh oh!

rxin commented Nov 2, 2014

Uh oh!

SparkQA commented Nov 2, 2014

Uh oh!

SparkQA commented Nov 2, 2014

Uh oh!

AmplabJenkins commented Nov 2, 2014

Uh oh!

rxin commented Nov 2, 2014

Uh oh!

SparkQA commented Nov 2, 2014

Uh oh!

SparkQA commented Nov 2, 2014

Uh oh!

AmplabJenkins commented Nov 2, 2014

Uh oh!

rxin commented Nov 2, 2014

Uh oh!

marmbrus commented Nov 2, 2014

Uh oh!

davies commented Nov 3, 2014

Uh oh!

rxin commented Nov 3, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants