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: [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]
