You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/07/12 19:51:58 UTC
[GitHub] [spark] Yikun commented on a change in pull request #33261: [SPARK-35334][K8S] Make Spark more resilient to intermittent K8s flakiness
Yikun commented on a change in pull request #33261:
URL: https://github.com/apache/spark/pull/33261#discussion_r667965962
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/SparkKubernetesClientFactory.scala
##########
@@ -74,6 +77,11 @@ private[spark] object SparkKubernetesClientFactory extends Logging {
kubeContext.map("context " + _).getOrElse("current context") +
" from users K8S config file")
+ // if backoff limit is not set then set it to 3
+ if (getSystemPropertyOrEnvVar(KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY) == null) {
+ System.setProperty(KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY, "3")
+ }
+
Review comment:
The [autoConfigure](https://github.com/fabric8io/kubernetes-client/blob/4e4dcc73bfd6e0d811e3aba32ffa7956b71d951d/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java#L418) helps us set backofflimit with [priority](https://github.com/fabric8io/kubernetes-client/blob/5bfcac2d8a144f66b1323df5988dd2761fc7e561/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/Utils.java#L80-L92) :
1. System.getProperty(KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY)
2. System.getenv(KUBERNETES_REQUEST_RETRY_BACKOFFLIMIT_SYSTEM_PROPERTY)
3. then [config.getRequestRetryBackoffLimit()](https://github.com/fabric8io/kubernetes-client/blob/4e4dcc73bfd6e0d811e3aba32ffa7956b71d951d/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/Config.java#L418)
So, it works, but looks like a little bit flaky for me (it dependents more internal implements of k8s-client), how about respects user property/env/config after config build() like:
```
if config.getRequestRetryBackoffLimit() != null:
config.setRequestRetryBackoffLimit("3")
```
##########
File path: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/SparkKubernetesClientFactory.scala
##########
@@ -102,6 +110,8 @@ private[spark] object SparkKubernetesClientFactory extends Logging {
val httpClientWithCustomDispatcher = baseHttpClient.newBuilder()
.dispatcher(dispatcher)
.build()
+ logDebug("Kubernetes client config: " +
Review comment:
nit: I think the log can be moved before L109(`HttpClientUtils.createHttpClient(config)`)
--
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