martin-g commented on a change in pull request #34035:
URL: https://github.com/apache/spark/pull/34035#discussion_r803405302
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -616,6 +616,20 @@ private[spark] object Config extends Logging {
.checkValue(value => value > 0, "Maximum number of pending pods should
be a positive integer")
.createWithDefault(Int.MaxValue)
+ val KUBERNETES_LOG_TO_FILE =
+ ConfigBuilder("spark.kubernetes.logToFile.enabled")
+ .doc("Whether to write executor/driver stdout/stderr as log file")
+ .version("3.2.0")
Review comment:
The version specified in `docs/running-on-kubernetes.md` says `3.3.0`.
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Config.scala
##########
@@ -616,6 +616,20 @@ private[spark] object Config extends Logging {
.checkValue(value => value > 0, "Maximum number of pending pods should
be a positive integer")
.createWithDefault(Int.MaxValue)
+ val KUBERNETES_LOG_TO_FILE =
+ ConfigBuilder("spark.kubernetes.logToFile.enabled")
+ .doc("Whether to write executor/driver stdout/stderr as log file")
+ .version("3.2.0")
+ .booleanConf
+ .createWithDefault(false)
+
+ val KUBERNETES_LOG_TO_FILE_PATH =
+ ConfigBuilder("spark.kubernetes.logToFile.path")
+ .doc("The path to write executor/driver stdout/stderr as log file")
+ .version("3.2.0")
Review comment:
ditto
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
##########
@@ -177,7 +177,14 @@ private[spark] class BasicExecutorFeatureStep(
.withValue(opt)
.build()
}
- }
+ } ++ {
+ if (kubernetesConf.get(KUBERNETES_LOG_TO_FILE)) {
+ Seq(new EnvVarBuilder()
+ .withName(ENV_SPARK_LOG_PATH)
+ .withValue(kubernetesConf.get(KUBERNETES_LOG_TO_FILE_PATH))
+ .build())
+ } else None
Review comment:
ditto
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala
##########
@@ -82,7 +82,14 @@ private[spark] class BasicDriverFeatureStep(conf:
KubernetesDriverConf)
.withName(env._1)
.withValue(env._2)
.build()
- }
+ } ++ {
+ if (conf.get(KUBERNETES_LOG_TO_FILE)) {
+ Seq(new EnvVarBuilder()
+ .withName(ENV_SPARK_LOG_PATH)
+ .withValue(conf.get(KUBERNETES_LOG_TO_FILE_PATH))
+ .build())
+ } else None
Review comment:
Does this compile ? `None` (Option) is not a collection.
Maybe `Seq.empty` ?!
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/Constants.scala
##########
@@ -66,6 +66,7 @@ private[spark] object Constants {
val ENV_SPARK_CONF_DIR = "SPARK_CONF_DIR"
val ENV_SPARK_USER = "SPARK_USER"
val ENV_RESOURCE_PROFILE_ID = "SPARK_RESOURCE_PROFILE_ID"
+ val ENV_SPARK_LOG_PATH = "K8S_SPARK_LOG_PATH"
Review comment:
all other env vars start with `SPARK_`. For consistency, I think it
would be better to change this to `SPARK_K8S_LOG_PATH`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
