-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-22006][k8s] Support to configure max concurrent requests for fabric8 Kubernetes client via JAVA opts or envs #15480
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 2c5c9cd (Fri May 28 08:59:42 UTC 2021) ✅no warnings Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
5daa35d
to
8c7eca1
Compare
cc @tillrohrmann This PR is ready for review. Please have a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this PR @wangyang0918. I think your changes should work. What I am missing is the documentation for this feature. In line with this thought, maybe it is clearer if we introduce a Flink configuration option to configure this value. If set, it would overwrite the default value/value configured through some other means. What do you think?
Utils.getSystemPropertyOrEnvVar( | ||
Config.KUBERNETES_MAX_CONCURRENT_REQUESTS, | ||
String.valueOf(Config.DEFAULT_MAX_CONCURRENT_REQUESTS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering whether we shouldn't rather make it configurable via a Flink configuration option? That way it would also be documented for our users and easier to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since fabric8 kubernetes-client provides many internal advanced configuration properties[1], I do not want to introduce such a special case for kubernetes.max.concurrent.requests
. If you think it is a problem that we do not have the documentation, maybe we could add a section in native_kubernetes.md#Flink on Kubernetes Reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then let's add a section about how to configure the Fabric8 kube client to native_kubernetes.md
where we state how to set these experts options and also tell explicitly about KUBERNETES_MAX_CONCURRENT_REQUESTS
if users want to submit a lot of jobs.
@tillrohrmann I have pushed another commit for the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating this PR @wangyang0918. The changes look good to me. Merging this PR now.
…abric8 Kubernetes client via JAVA opts or envs After this commit, users could use any one in the following Flink config options to set the concurrent max requests, which allows to run more jobs in a session cluster. Please note that, each Flink job will consume 3 concurrent requests. * containerized.master.env.KUBERNETES_MAX_CONCURRENT_REQUESTS: 200 * env.java.opts.jobmanager: "-Dkubernetes.max.concurrent.requests=200"
…ubernetes client This closes apache#15480.
b866c12
to
e5d1788
Compare
…ubernetes client This closes apache#15480.
…ubernetes client This closes apache#15480.
e5d1788
to
2c5c9cd
Compare
…ubernetes client This closes #15480.
What is the purpose of the change
Flink now could only run 20 jobs or less in a K8s session(native and standalone) when K8s HA enabled.
The root cause is fabric8 Kubernetes client has configured the MaxRequests of
OkHttpClient#Dispatcher
[1] to 64[2], which means we could not create more than 64 watchers in the JobManager pod.Normally, it could be configured in Flink via env or java opts. Unfortunately, the fabric8 Kubernetes client version 4.9.2 has a bug[3], which causes the max concurrent requests could not be set via system properties.
This PR is a temporary fix before we bumping the fabric8 Kubernetes version to 4.13.0+ or change the behavior how we watch the leader ConfigMaps.
After this commit, users could use any one in the following Flink config options to set the concurrent max requests, which allows to run more jobs in a session cluster. Please note that, each Flink job will consume 3 concurrent requests.
containerized.master.env.KUBERNETES_MAX_CONCURRENT_REQUESTS: 200
env.java.opts.jobmanager: "-Dkubernetes.max.concurrent.requests=200"
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation