[spark] branch branch-2.4 updated: [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before real start

2020-09-21 Thread dongjoon
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
 new e1e94ed  [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task 
killed before real start
e1e94ed is described below

commit e1e94ed4ef45ef81814f1b920bac0afa52ae06a2
Author: yi.wu 
AuthorDate: Mon Sep 21 23:20:18 2020 -0700

[SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before 
real start

### What changes were proposed in this pull request?

Only calculate the executorRunTime when taskStartTime > 0. Otherwise, set 
executorRunTime to 0.

### Why are the changes needed?

bug fix.

It's possible that a task be killed (e.g., by another successful attempt) 
before it reaches "taskStartTime = System.currentTimeMillis()". In this case, 
taskStartTime is still 0 since it hasn't been really initialized. And we will 
get the wrong executorRunTime by calculating System.currentTimeMillis() - 
taskStartTime.

### Does this PR introduce _any_ user-facing change?

Yes, users will see the correct executorRunTime.

### How was this patch tested?

Pass existing tests.

Closes #29832 from Ngone51/backport-spark-3289.

Authored-by: yi.wu 
Signed-off-by: Dongjoon Hyun 
---
 core/src/main/scala/org/apache/spark/executor/Executor.scala | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/executor/Executor.scala 
b/core/src/main/scala/org/apache/spark/executor/Executor.scala
index f7ff0b8..fe57b1c 100644
--- a/core/src/main/scala/org/apache/spark/executor/Executor.scala
+++ b/core/src/main/scala/org/apache/spark/executor/Executor.scala
@@ -337,7 +337,10 @@ private[spark] class Executor(
 private def collectAccumulatorsAndResetStatusOnFailure(taskStartTime: 
Long) = {
   // Report executor runtime and JVM gc time
   Option(task).foreach(t => {
-t.metrics.setExecutorRunTime(System.currentTimeMillis() - 
taskStartTime)
+t.metrics.setExecutorRunTime(
+  // SPARK-32898: it's possible that a task is killed when 
taskStartTime has the initial
+  // value(=0) still. In this case, the executorRunTime should be 
considered as 0.
+  if (taskStartTime > 0) System.currentTimeMillis() - taskStartTime 
else 0)
 t.metrics.setJvmGCTime(computeTotalGcTime() - startGCTime)
   })
 


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



[spark] branch branch-2.4 updated: [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before real start

2020-09-21 Thread dongjoon
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
 new e1e94ed  [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task 
killed before real start
e1e94ed is described below

commit e1e94ed4ef45ef81814f1b920bac0afa52ae06a2
Author: yi.wu 
AuthorDate: Mon Sep 21 23:20:18 2020 -0700

[SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before 
real start

### What changes were proposed in this pull request?

Only calculate the executorRunTime when taskStartTime > 0. Otherwise, set 
executorRunTime to 0.

### Why are the changes needed?

bug fix.

It's possible that a task be killed (e.g., by another successful attempt) 
before it reaches "taskStartTime = System.currentTimeMillis()". In this case, 
taskStartTime is still 0 since it hasn't been really initialized. And we will 
get the wrong executorRunTime by calculating System.currentTimeMillis() - 
taskStartTime.

### Does this PR introduce _any_ user-facing change?

Yes, users will see the correct executorRunTime.

### How was this patch tested?

Pass existing tests.

Closes #29832 from Ngone51/backport-spark-3289.

Authored-by: yi.wu 
Signed-off-by: Dongjoon Hyun 
---
 core/src/main/scala/org/apache/spark/executor/Executor.scala | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/executor/Executor.scala 
b/core/src/main/scala/org/apache/spark/executor/Executor.scala
index f7ff0b8..fe57b1c 100644
--- a/core/src/main/scala/org/apache/spark/executor/Executor.scala
+++ b/core/src/main/scala/org/apache/spark/executor/Executor.scala
@@ -337,7 +337,10 @@ private[spark] class Executor(
 private def collectAccumulatorsAndResetStatusOnFailure(taskStartTime: 
Long) = {
   // Report executor runtime and JVM gc time
   Option(task).foreach(t => {
-t.metrics.setExecutorRunTime(System.currentTimeMillis() - 
taskStartTime)
+t.metrics.setExecutorRunTime(
+  // SPARK-32898: it's possible that a task is killed when 
taskStartTime has the initial
+  // value(=0) still. In this case, the executorRunTime should be 
considered as 0.
+  if (taskStartTime > 0) System.currentTimeMillis() - taskStartTime 
else 0)
 t.metrics.setJvmGCTime(computeTotalGcTime() - startGCTime)
   })
 


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



[spark] branch branch-2.4 updated: [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before real start

2020-09-21 Thread dongjoon
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
 new e1e94ed  [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task 
killed before real start
e1e94ed is described below

commit e1e94ed4ef45ef81814f1b920bac0afa52ae06a2
Author: yi.wu 
AuthorDate: Mon Sep 21 23:20:18 2020 -0700

[SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before 
real start

### What changes were proposed in this pull request?

Only calculate the executorRunTime when taskStartTime > 0. Otherwise, set 
executorRunTime to 0.

### Why are the changes needed?

bug fix.

It's possible that a task be killed (e.g., by another successful attempt) 
before it reaches "taskStartTime = System.currentTimeMillis()". In this case, 
taskStartTime is still 0 since it hasn't been really initialized. And we will 
get the wrong executorRunTime by calculating System.currentTimeMillis() - 
taskStartTime.

### Does this PR introduce _any_ user-facing change?

Yes, users will see the correct executorRunTime.

### How was this patch tested?

Pass existing tests.

Closes #29832 from Ngone51/backport-spark-3289.

Authored-by: yi.wu 
Signed-off-by: Dongjoon Hyun 
---
 core/src/main/scala/org/apache/spark/executor/Executor.scala | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/executor/Executor.scala 
b/core/src/main/scala/org/apache/spark/executor/Executor.scala
index f7ff0b8..fe57b1c 100644
--- a/core/src/main/scala/org/apache/spark/executor/Executor.scala
+++ b/core/src/main/scala/org/apache/spark/executor/Executor.scala
@@ -337,7 +337,10 @@ private[spark] class Executor(
 private def collectAccumulatorsAndResetStatusOnFailure(taskStartTime: 
Long) = {
   // Report executor runtime and JVM gc time
   Option(task).foreach(t => {
-t.metrics.setExecutorRunTime(System.currentTimeMillis() - 
taskStartTime)
+t.metrics.setExecutorRunTime(
+  // SPARK-32898: it's possible that a task is killed when 
taskStartTime has the initial
+  // value(=0) still. In this case, the executorRunTime should be 
considered as 0.
+  if (taskStartTime > 0) System.currentTimeMillis() - taskStartTime 
else 0)
 t.metrics.setJvmGCTime(computeTotalGcTime() - startGCTime)
   })
 


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



[spark] branch branch-2.4 updated: [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before real start

2020-09-21 Thread dongjoon
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
 new e1e94ed  [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task 
killed before real start
e1e94ed is described below

commit e1e94ed4ef45ef81814f1b920bac0afa52ae06a2
Author: yi.wu 
AuthorDate: Mon Sep 21 23:20:18 2020 -0700

[SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before 
real start

### What changes were proposed in this pull request?

Only calculate the executorRunTime when taskStartTime > 0. Otherwise, set 
executorRunTime to 0.

### Why are the changes needed?

bug fix.

It's possible that a task be killed (e.g., by another successful attempt) 
before it reaches "taskStartTime = System.currentTimeMillis()". In this case, 
taskStartTime is still 0 since it hasn't been really initialized. And we will 
get the wrong executorRunTime by calculating System.currentTimeMillis() - 
taskStartTime.

### Does this PR introduce _any_ user-facing change?

Yes, users will see the correct executorRunTime.

### How was this patch tested?

Pass existing tests.

Closes #29832 from Ngone51/backport-spark-3289.

Authored-by: yi.wu 
Signed-off-by: Dongjoon Hyun 
---
 core/src/main/scala/org/apache/spark/executor/Executor.scala | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/executor/Executor.scala 
b/core/src/main/scala/org/apache/spark/executor/Executor.scala
index f7ff0b8..fe57b1c 100644
--- a/core/src/main/scala/org/apache/spark/executor/Executor.scala
+++ b/core/src/main/scala/org/apache/spark/executor/Executor.scala
@@ -337,7 +337,10 @@ private[spark] class Executor(
 private def collectAccumulatorsAndResetStatusOnFailure(taskStartTime: 
Long) = {
   // Report executor runtime and JVM gc time
   Option(task).foreach(t => {
-t.metrics.setExecutorRunTime(System.currentTimeMillis() - 
taskStartTime)
+t.metrics.setExecutorRunTime(
+  // SPARK-32898: it's possible that a task is killed when 
taskStartTime has the initial
+  // value(=0) still. In this case, the executorRunTime should be 
considered as 0.
+  if (taskStartTime > 0) System.currentTimeMillis() - taskStartTime 
else 0)
 t.metrics.setJvmGCTime(computeTotalGcTime() - startGCTime)
   })
 


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



[spark] branch branch-2.4 updated: [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before real start

2020-09-21 Thread dongjoon
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
 new e1e94ed  [SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task 
killed before real start
e1e94ed is described below

commit e1e94ed4ef45ef81814f1b920bac0afa52ae06a2
Author: yi.wu 
AuthorDate: Mon Sep 21 23:20:18 2020 -0700

[SPARK-32898][2.4][CORE] Fix wrong executorRunTime when task killed before 
real start

### What changes were proposed in this pull request?

Only calculate the executorRunTime when taskStartTime > 0. Otherwise, set 
executorRunTime to 0.

### Why are the changes needed?

bug fix.

It's possible that a task be killed (e.g., by another successful attempt) 
before it reaches "taskStartTime = System.currentTimeMillis()". In this case, 
taskStartTime is still 0 since it hasn't been really initialized. And we will 
get the wrong executorRunTime by calculating System.currentTimeMillis() - 
taskStartTime.

### Does this PR introduce _any_ user-facing change?

Yes, users will see the correct executorRunTime.

### How was this patch tested?

Pass existing tests.

Closes #29832 from Ngone51/backport-spark-3289.

Authored-by: yi.wu 
Signed-off-by: Dongjoon Hyun 
---
 core/src/main/scala/org/apache/spark/executor/Executor.scala | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/core/src/main/scala/org/apache/spark/executor/Executor.scala 
b/core/src/main/scala/org/apache/spark/executor/Executor.scala
index f7ff0b8..fe57b1c 100644
--- a/core/src/main/scala/org/apache/spark/executor/Executor.scala
+++ b/core/src/main/scala/org/apache/spark/executor/Executor.scala
@@ -337,7 +337,10 @@ private[spark] class Executor(
 private def collectAccumulatorsAndResetStatusOnFailure(taskStartTime: 
Long) = {
   // Report executor runtime and JVM gc time
   Option(task).foreach(t => {
-t.metrics.setExecutorRunTime(System.currentTimeMillis() - 
taskStartTime)
+t.metrics.setExecutorRunTime(
+  // SPARK-32898: it's possible that a task is killed when 
taskStartTime has the initial
+  // value(=0) still. In this case, the executorRunTime should be 
considered as 0.
+  if (taskStartTime > 0) System.currentTimeMillis() - taskStartTime 
else 0)
 t.metrics.setJvmGCTime(computeTotalGcTime() - startGCTime)
   })
 


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org