gianm commented on code in PR #18540:
URL: https://github.com/apache/druid/pull/18540#discussion_r2355194509
##########
extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesOverlordModule.java:
##########
@@ -79,7 +85,10 @@ public class KubernetesOverlordModule implements DruidModule
+
".k8sAndWorker";
private static final String RUNNERSTRATEGY_PROPERTIES_FORMAT_STRING =
K8SANDWORKER_PROPERTIES_PREFIX
+
".runnerStrategy.%s";
- private static final String HTTPCLIENT_PROPERITES_PREFIX =
K8SANDWORKER_PROPERTIES_PREFIX + ".http";
+ private static final String HTTPCLIENT_TYPE_PROPERTY =
K8SANDWORKER_PROPERTIES_PREFIX + ".http.httpClientType";
Review Comment:
You haven't added documentation for this parameter, which I think is good
because I think it's a parameter that we may remove "soon". I suggest noting
that in a comment here. Its purpose is to help cluster operators evaluate
different http clients with fabric8, and we're adding it because we've noticed
issues with two of the major ones:
- the old default (okhttp) can lead to excessive numbers of threads if you
have a lot of tasks running
- the new default (vert.x) has been observed to generate spurious API call
failures, which leads to seemingly-random task failures
Ideally, through some evaluation we will determine which one is best to use
and how best to configure it, and at that point we could remove the property.
It will help us slim down the distribution since it won't need to ship all 3
fabric8 client modules.
--
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]
