[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/19132 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user caneGuy commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138811435 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -69,7 +70,8 @@ private[v1] object AllStagesResource { } val taskData = if (includeDetails) { - Some(stageUiData.taskData.map { case (k, v) => k -> convertTaskData(v) } ) + Some(stageUiData.taskData.map { case (k, v) => +k -> convertTaskData(v, stageUiData.lastUpdateTime) } ) --- End diff -- Thanks for your patient.Actually i did not check carefully. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138805063 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -69,7 +70,8 @@ private[v1] object AllStagesResource { } val taskData = if (includeDetails) { - Some(stageUiData.taskData.map { case (k, v) => k -> convertTaskData(v) } ) + Some(stageUiData.taskData.map { case (k, v) => +k -> convertTaskData(v, stageUiData.lastUpdateTime) } ) --- End diff -- Sorry I may stated clearly, it should be: ``` Some(stageUiData.taskData.map { case (k, v) => k -> convertTaskData(v, stageUiData.lastUpdateTime) }) ``` No whitespace after "}". --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138789492 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/OneStageResource.scala --- @@ -81,7 +83,8 @@ private[v1] class OneStageResource(ui: SparkUI) { @DefaultValue("20") @QueryParam("length") length: Int, @DefaultValue("ID") @QueryParam("sortBy") sortBy: TaskSorting): Seq[TaskData] = { withStageAttempt(stageId, stageAttemptId) { stage => - val tasks = stage.ui.taskData.values.map{AllStagesResource.convertTaskData}.toIndexedSeq + val tasks = stage.ui.taskData.values.map{ --- End diff -- The style should be changed to `map { AllStagesResource.convertTaskData(_, ui.lastUpdateTime) }`, requires whitespace between `{` and `}`. You could check other similar codes about the style. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138789213 --- Diff: core/src/main/scala/org/apache/spark/ui/jobs/UIData.scala --- @@ -97,6 +97,7 @@ private[spark] object UIData { var memoryBytesSpilled: Long = _ var diskBytesSpilled: Long = _ var isBlacklisted: Int = _ +var jobLastUpdateTime: Option[Long] = None --- End diff -- Is it better to rename to `stageLastUpdateTime` or just `lastUpdateTime`? Since this structure unrelated to job, would be better to not involve "job". --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138533547 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -47,7 +47,8 @@ private[v1] class AllStagesResource(ui: SparkUI) { listener.stageIdToData.get((stageInfo.stageId, stageInfo.attemptId)) } } yield { - AllStagesResource.stageUiToStageData(status, stageInfo, stageUiData, includeDetails = false) + AllStagesResource.stageUiToStageData( +status, stageInfo, stageUiData, includeDetails = false, Some(ui)) --- End diff -- It's not a good idea to pass in `SparkUI` to only get `lastUpdate`, the API looks weird to add this `SparkUI` argument, the fix here only just make it work. It is better to add one more field in `StageUIData` or `TaskUIData` if possible. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138526050 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- Yes, if it is not a big change I think it should be fixed here. Because currently with this fix UI and REST API are inconsistent. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user caneGuy commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138523362 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- You are right, @jerryshao .IIUC, the `ui` in `AllStagesResource.scala` is passed from `ApiRootResource` which also create `sparkUI` by `FSHistoryProvider`.So we can also get `lastUpdateTime` from this `ui` in `AllStagesResource` and pass to the `taskDuration` interface.I think it is another problem for REST?Should we fix here? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138510683 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- Here what if we call the REST API on history server to get stage info? Looks like we may still have this issue since we don't have last update time here, what do you think @ajbozarth ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user caneGuy commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138243757 --- Diff: core/src/main/scala/org/apache/spark/ui/SparkUI.scala --- @@ -50,6 +50,7 @@ private[spark] class SparkUI private ( val operationGraphListener: RDDOperationGraphListener, var appName: String, val basePath: String, +val lastUpdateTime: Long = -1L, --- End diff -- Update @jerryshao Thanks for your time. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user jerryshao commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r138240053 --- Diff: core/src/main/scala/org/apache/spark/ui/SparkUI.scala --- @@ -50,6 +50,7 @@ private[spark] class SparkUI private ( val operationGraphListener: RDDOperationGraphListener, var appName: String, val basePath: String, +val lastUpdateTime: Long = -1L, --- End diff -- I would like to user `Option[Long] = None` as default value to reflect there's no update time. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user caneGuy commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r137255922 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- Thanks @ajbozarth --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user ajbozarth commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r137238007 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- odd, I thought for sure it'd be fine, then this LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user caneGuy commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r137214624 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- Since i changed `taskDuration` below,if do not add `()` a compiler error will occur. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
Github user ajbozarth commented on a diff in the pull request: https://github.com/apache/spark/pull/19132#discussion_r137195530 --- Diff: core/src/main/scala/org/apache/spark/status/api/v1/AllStagesResource.scala --- @@ -142,7 +142,7 @@ private[v1] object AllStagesResource { index = uiData.taskInfo.index, attempt = uiData.taskInfo.attemptNumber, launchTime = new Date(uiData.taskInfo.launchTime), - duration = uiData.taskDuration, + duration = uiData.taskDuration(), --- End diff -- this is unrelated and unnecessary --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #19132: [SPARK-21922] Fix duration always updating when t...
GitHub user caneGuy opened a pull request: https://github.com/apache/spark/pull/19132 [SPARK-21922] Fix duration always updating when task failed but status is still RUN⦠â¦NING ## What changes were proposed in this pull request? When executor failed and task metrics have not send to driver,the status will always be 'RUNNING' and the duration will be 'CurrentTime - launchTime'. We can fix this time by modify time of event log. And the result picture is uploaded in [SPARK-21922](https://issues.apache.org/jira/browse/SPARK-21922) ## How was this patch tested? Deploy historyserver and open a same job. You can merge this pull request into a Git repository by running: $ git pull https://github.com/caneGuy/spark zhoukang/fix-duration Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/19132.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 #19132 commit 03a33a96c3cae0e4272a7bae2230f3a8c2c4589a Author: zhoukangDate: 2017-09-05T10:28:06Z Fix duration always updating when task failed but status is still RUNNING --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org