You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dongjoon-hyun (via GitHub)" <gi...@apache.org> on 2023/08/07 18:44:19 UTC
[GitHub] [spark] dongjoon-hyun opened a new pull request, #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
dongjoon-hyun opened a new pull request, #42381:
URL: https://github.com/apache/spark/pull/42381
### What changes were proposed in this pull request?
This PR aims to use `INFO`-level log instead of `WARN`-level in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped. Since Spark can distinguish the expected behavior from the error cases, Spark had better avoid WARNING.
### Why are the changes needed?
Previously, we have `WARN ExecutorPodsWatchSnapshotSource: Kubernetes client has been closed` message.
```
23/08/07 18:10:14 INFO SparkContext: SparkContext is stopping with exitCode 0.
23/08/07 18:10:14 WARN TaskSetManager: Lost task 2594.0 in stage 0.0 (TID 2594) ([2620:149:100d:1813::3f86] executor 1615): TaskKilled (another attempt succeeded)
23/08/07 18:10:14 INFO TaskSetManager: task 2594.0 in stage 0.0 (TID 2594) failed, but the task will not be re-executed (either because the task failed with a shuffle data fetch failure, so the previous stage needs to be re-run, or because a different copy of the task has already succeeded).
23/08/07 18:10:14 INFO SparkUI: Stopped Spark web UI at http://xxx:4040
23/08/07 18:10:14 INFO KubernetesClusterSchedulerBackend: Shutting down all executors
23/08/07 18:10:14 INFO KubernetesClusterSchedulerBackend$KubernetesDriverEndpoint: Asking each executor to shut down
23/08/07 18:10:14 WARN ExecutorPodsWatchSnapshotSource: Kubernetes client has been closed.
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass the CIs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
URL: https://github.com/apache/spark/pull/42381
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286374663
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
Review Comment:
Oh okay, if it is only case `getActive` return None, then it is fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286365074
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
Review Comment:
Do you mean `getOrElse(false)`?
Here, when `SparkContext.getActive` is `None`, it should be considered as `stopped`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286359766
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
Review Comment:
Should we use false as default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286270121
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
+ logInfo("Kubernetes client has been closed.")
Review Comment:
In this case, we change the log message and also ignore the exception details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286375435
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
Review Comment:
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42381:
URL: https://github.com/apache/spark/pull/42381#issuecomment-1668567470
Merged to master for Apache Spark 4.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #42381:
URL: https://github.com/apache/spark/pull/42381#discussion_r1286269707
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsWatchSnapshotSource.scala:
##########
@@ -86,12 +86,20 @@ class ExecutorPodsWatchSnapshotSource(
}
override def onClose(e: WatcherException): Unit = {
- logWarning("Kubernetes client has been closed (this is expected if the application is" +
- " shutting down.)", e)
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
+ logInfo("Kubernetes client has been closed.")
+ } else {
+ logWarning("Kubernetes client has been closed (this is expected if the application is" +
+ " shutting down.)", e)
+ }
}
override def onClose(): Unit = {
- logWarning("Kubernetes client has been closed.")
+ if (SparkContext.getActive.map(_.isStopped).getOrElse(true)) {
+ logInfo("Kubernetes client has been closed.")
Review Comment:
In this case, we only switch log-levels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #42381: [SPARK-44707][K8S] Use INFO log in `ExecutorPodsWatcher.onClose` if `SparkContext` is stopped
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #42381:
URL: https://github.com/apache/spark/pull/42381#issuecomment-1668510648
Could you review this log-related PR when you have some time, @viirya ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org