You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by advancedxy <gi...@git.apache.org> on 2018/01/07 16:02:01 UTC
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
GitHub user advancedxy opened a pull request:
https://github.com/apache/spark/pull/20178
[Spark-22952][CORE] Deprecate stageAttemptId in favour of stageAttemptNumber
## What changes were proposed in this pull request?
1. Deprecate attemptId in StageInfo and add `def attemptNumber() = attemptId`
2. Replace usage of stageAttemptId with stageAttemptNumber
## How was this patch tested?
I manually checked the compiler warning info
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/advancedxy/spark SPARK-22952
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20178.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20178
----
commit 4b18ea4f109b0f8d865e3fff8f415d87db528fa5
Author: Xianjin YE <ad...@...>
Date: 2018-01-07T15:17:16Z
Deprecate attemptId and in favour of attemptNumber
commit 2ec919761ced379f00e1fa9804a66e3b15e9d2e9
Author: Xianjin YE <ad...@...>
Date: 2018-01-07T15:43:28Z
Replace usage of stageAttemptId with stageAttemptNumber
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20178
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20178
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20178
thanks, merging to master/2.3!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20178
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85785/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160076144
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
how about
```
class StageInfo(
...
val attempNumber: Int,
... {
@deprecated
def attempId: Int = attemptNumber
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20178
**[Test build #85785 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85785/testReport)** for PR 20178 at commit [`2ec9197`](https://github.com/apache/spark/commit/2ec919761ced379f00e1fa9804a66e3b15e9d2e9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160087405
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
It may break the json protocol?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20178
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20178
add to whilelist
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160173491
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
I'd like to add a constructor and fix the json parser, otherwise we are not fully deprecating it. But let's do it in a new PR, as it may need quite a lot of changes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160170753
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
So, keep it this way or add a constructor and fix Json parser?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on the issue:
https://github.com/apache/spark/pull/20178
cc @cloud-fan @zsxwing and @squito
I only included the changes related to StageInfo's deprecated getter. It would involve too much changes if we want to replace attemptId with attemptNumber everywhere, which I think we should do that until we are targeting Spark 3.0.
I even think some of the changes in this patch may not be applied as mixed `attemptId` and `attemptNumber` appears in the same context.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on the issue:
https://github.com/apache/spark/pull/20178
Please add me to whitelist...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20178
**[Test build #85785 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85785/testReport)** for PR 20178 at commit [`2ec9197`](https://github.com/apache/spark/commit/2ec919761ced379f00e1fa9804a66e3b15e9d2e9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160236611
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
Oh, I was wrong. It seems the rest api doesn't use this class. It's using `org.apache.spark.status.api.v1.StageData`. However, it's also called `attemptId` in this class.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160093005
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
So we do not fully deprecate it, it's still `attemptId` in json. Shall we fix the json too? We may need to add some logic at the parser side to recognize `attemptId`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20178: [Spark-22952][CORE] Deprecate stageAttemptId in favour o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20178
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160076700
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
hmmm, can we add a new constructor for `attemptId`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160077159
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
hmmmm, maybe.. Let me try it out after work...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160587943
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
> But let's do it in a new PR, as it may need quite a lot of changes.
Lets's do that after Spark 2.3 release then?
>However, it's also called attemptId in this class.
Yeah, (stage)attemptId is over a lot of places...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160076571
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
ah i see, let's keep it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20178: [Spark-22952][CORE] Deprecate stageAttemptId in f...
Posted by advancedxy <gi...@git.apache.org>.
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20178#discussion_r160076462
--- Diff: core/src/main/scala/org/apache/spark/scheduler/StageInfo.scala ---
@@ -56,6 +56,8 @@ class StageInfo(
completionTime = Some(System.currentTimeMillis)
}
+ def attemptNumber(): Int = attemptId
--- End diff --
If we go this way, I believe it would break source compatibility for Developer API.
Code like `new StageInfo(stageId = xx, attemptId = yy, ...)` couldn't by compiled any more.
Not sure about binary compatibility
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org