[SPARK-3543] Write TaskContext in Java and expose it through a static accessor. #2425

ScrapCodes · 2014-09-17T08:19:48Z

No description provided.

rxin · 2014-09-17T08:22:59Z

core/src/main/java/org/apache/spark/TaskContext.java

+    }
+
+
+    private static ThreadLocal<TaskContext> taskContext =


u don't need to wrap this, do u?

You mean text wrap ?, yes in one line they are 114 chars.

SparkQA · 2014-09-17T08:24:19Z

QA tests have started for PR 2425 at commit f716fd1.

This patch merges cleanly.

rxin · 2014-09-17T08:25:01Z

core/src/main/java/org/apache/spark/TaskContext.java

+        List<TaskCompletionListener> revlist =
+                new ArrayList<TaskCompletionListener>(onCompleteCallbacks);
+        Collections.reverse(revlist);
+        for (TaskCompletionListener tcl : revlist){


space before {

ScrapCodes · 2014-09-17T09:06:56Z

@rxin One side question, Java8ApiSuite(s) don't compile, looks like we have been overlooking them for a while. May be we could just remove them ?

SparkQA · 2014-09-17T09:19:27Z

QA tests have started for PR 2425 at commit edf945e.

This patch merges cleanly.

SparkQA · 2014-09-17T09:30:52Z

QA tests have finished for PR 2425 at commit f716fd1.

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

SparkQA · 2014-09-17T10:04:19Z

QA tests have started for PR 2425 at commit a7d5e23.

This patch merges cleanly.

SparkQA · 2014-09-17T10:28:12Z

QA tests have finished for PR 2425 at commit edf945e.

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

SparkQA · 2014-09-17T11:11:33Z

QA tests have finished for PR 2425 at commit a7d5e23.

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

rxin · 2014-09-17T17:15:59Z

core/src/main/java/org/apache/spark/TaskContext.java

+    interrupted = true;
+  }
+
+  public Integer stageId() {


int? why box?

SparkQA · 2014-09-18T10:04:38Z

QA tests have started for PR 2425 at commit ee8bd00.

This patch merges cleanly.

SparkQA · 2014-09-18T11:14:17Z

QA tests have finished for PR 2425 at commit ee8bd00.

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

pwendell · 2014-09-21T01:35:09Z

core/src/main/scala/org/apache/spark/scheduler/Task.scala

@@ -45,7 +45,8 @@ import org.apache.spark.util.Utils
 private[spark] abstract class Task[T](val stageId: Int, var partitionId: Int) extends Serializable {

  final def run(attemptId: Long): T = {
-    context = new TaskContext(stageId, partitionId, attemptId, runningLocally = false)
+    context = new TaskContext(stageId, partitionId, attemptId, false)


any reason to drop the naming for runningLocally? the correct style is to name all optional paremeters.

if the parameter is defined in java it cannot be named

pwendell · 2014-09-21T01:55:00Z

@ScrapCodes made another pass with some comments. Overall this is looking good

pwendell · 2014-09-25T05:53:37Z

@ScrapCodes will you have time to address the feedback?

SparkQA · 2014-09-26T10:24:24Z

QA tests have started for PR 2425 at commit 155bc3b.

This patch merges cleanly.

SparkQA · 2014-09-26T10:27:46Z

QA tests have finished for PR 2425 at commit 155bc3b.

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

AmplabJenkins · 2014-09-26T10:27:47Z

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

SparkQA · 2014-09-26T10:39:24Z

QA tests have started for PR 2425 at commit 5f8555b.

This patch merges cleanly.

SparkQA · 2014-09-26T10:42:47Z

QA tests have finished for PR 2425 at commit 5f8555b.

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

AmplabJenkins · 2014-09-26T10:42:48Z

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

SparkQA · 2014-09-26T10:49:20Z

QA tests have started for PR 2425 at commit 8ae414c.

This patch merges cleanly.

SparkQA · 2014-09-26T11:57:53Z

QA tests have finished for PR 2425 at commit 8ae414c.

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

AmplabJenkins · 2014-09-26T11:57:56Z

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

rxin · 2014-09-27T04:29:38Z

Ok I'm merging this. I will push some minor cleanups after merging.

rxin · 2014-09-27T04:32:51Z

core/src/main/java/org/apache/spark/TaskContext.java

+   * @param taskMetrics performance metrics of the task
+   */
+  @DeveloperApi
+  public TaskContext(Integer stageId, Integer partitionId, Long attemptId, Boolean runningLocally,


fyi all these should be primitives rather than boxed Integers/Longs/Booleans. I'm going to fix it in a separate pr.

This addresses some minor issues in #2425 Author: Reynold Xin <rxin@apache.org> Closes #2557 from rxin/TaskContext and squashes the following commits: a51e5f6 [Reynold Xin] [SPARK-3543] Clean up Java TaskContext implementation.

ScrapCodes added 2 commits September 17, 2014 13:10

Translated Task context from scala to java.

333c7d6

introduced thread local for getting the task context.

f716fd1

rxin reviewed Sep 17, 2014
View reviewed changes

Code review git add -A

edf945e

Added doc comments.

a7d5e23

rxin reviewed Sep 17, 2014
View reviewed changes

core/src/main/java/org/apache/spark/TaskContext.java

interrupted = true;

}

public Integer stageId() {

Copy link

Contributor

rxin Sep 17, 2014

Choose a reason for hiding this comment

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

int? why box?

ScrapCodes added 2 commits September 18, 2014 15:28

Moved setting the thread local to where TaskContext is instantiated.

ddb8cbe

Added internal API in docs comments.

ee8bd00

pwendell reviewed Sep 21, 2014
View reviewed changes

ScrapCodes force-pushed the SPARK-3543/withTaskContext branch from 155bc3b to 5f8555b Compare September 26, 2014 10:32

CR

8ae414c

ScrapCodes force-pushed the SPARK-3543/withTaskContext branch from 5f8555b to 8ae414c Compare September 26, 2014 10:44

asfgit closed this in 5e34855 Sep 27, 2014

rxin reviewed Sep 27, 2014
View reviewed changes

This was referenced Sep 27, 2014

[SPARK-3543] Clean up Java TaskContext implementation. #2557

Closed

SPARK-2895: Add mapPartitionsWithContext related support on Spark Java API. #2194

Closed

ScrapCodes deleted the SPARK-3543/withTaskContext branch June 3, 2015 05:58

[SPARK-3543] Write TaskContext in Java and expose it through a static accessor. #2425

[SPARK-3543] Write TaskContext in Java and expose it through a static accessor. #2425

Uh oh!

Conversation

ScrapCodes commented Sep 17, 2014

Uh oh!

rxin Sep 17, 2014

Choose a reason for hiding this comment

Uh oh!

ScrapCodes Sep 17, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

rxin Sep 17, 2014

Choose a reason for hiding this comment

Uh oh!

ScrapCodes commented Sep 17, 2014

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

SparkQA commented Sep 17, 2014

Uh oh!

rxin Sep 17, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 18, 2014

Uh oh!

SparkQA commented Sep 18, 2014

Uh oh!

pwendell Sep 21, 2014

Choose a reason for hiding this comment

Uh oh!

rxin Sep 27, 2014

Choose a reason for hiding this comment

Uh oh!

pwendell commented Sep 21, 2014

Uh oh!

pwendell commented Sep 25, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

AmplabJenkins commented Sep 26, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

AmplabJenkins commented Sep 26, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

SparkQA commented Sep 26, 2014

Uh oh!

AmplabJenkins commented Sep 26, 2014

Uh oh!

rxin commented Sep 27, 2014

Uh oh!

rxin Sep 27, 2014

Choose a reason for hiding this comment

Uh oh!

Uh oh!