You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/02/17 12:06:46 UTC

[GitHub] [flink] xintongsong opened a new pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

xintongsong opened a new pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110
 
 
   ## What is the purpose of the change
   
   This PR fix that Kubernetes deployment not respecting 'taskmanager.cpu.core'.
   The expected behavior for cpu configuration is to fallback the cpu configuration with the following order:
   1. common cpu config option ('taskmanager.cpu.core')
   2. K8s/Yarn/Mesos config option ('kubernetes.taskmanager.cpu' for Kubernetes)
   3. number of slots ('taskmanager.numberOfTaskSlots')
   
   ## Brief change log
   
   - a43e139a4ffeeb06b00db4163e329dfcbfbb9f3b: Make Kubernetes to respect 'taskmanager.cpu.cores'.
   - 9db123635d9b3142f3bf9b1b61347f7d46ca4890, c330e78bc64b398d2ff483ab3f996db3958ec2fd: Hotfixes for adding test cases validating the same behavior on Yarn/Mesos deployments.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   - Add test cases in `KubernetesResourceManagerTest`
   - Add test cases in `YarnResourceManagerTest`
   - Update existing test cases in `MesosTaskManagerParametersTest`
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (not applicable)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150291695",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/149298398) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253) 
   * 8481273b59d64a27b53ee4dc87454998a6e0b895 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150291695) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-590759872
 
 
   merged into master by 28365501edb2671efe6160c921325829d9e088d3
   merged into 1.10 by ac2aaf9277333a6d8ac5aa1c0c81189f56e6ffd4

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] wangyang0918 commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r380440613
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,7 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		double fallback = configuration.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU);
+		return TaskExecutorProcessUtils.getCpuCoresWithFallback(configuration, fallback).getValue().doubleValue();
 
 Review comment:
   Since K8s/Yarn/Mesos share the same logics to get cpu cores, maybe we could provide a unified method. Then we will not need to test for each implementation.
   ```
   public static <T> CPUResource getCpuCoresWithFallback(final Configuration config, ConfigOption<T> fallback)
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383207491
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,7 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		double fallback = configuration.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU);
+		return TaskExecutorProcessUtils.getCpuCoresWithFallback(configuration, fallback).getValue().doubleValue();
 
 Review comment:
   I agree with @azagrebin that it is difficult to unify Yarn with K8s/Mesos. We cannot easily get the type of the config option, which means it's hard to properly read the config option into a double value.
   
   I'll create a util method share by K8s/Mesos.
   
   Regarding testing, I think we need this test for each deployment anyway, to test the proper fallback of different config options on different deployment.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586965403
 
 
   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit c330e78bc64b398d2ff483ab3f996db3958ec2fd (Mon Feb 17 12:10:00 UTC 2020)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    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 <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383734545
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,6 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		return TaskExecutorProcessUtils.getCpuCoresWithFallbackConfigOption(configuration, KubernetesConfigOptions.TASK_MANAGER_CPU);
 
 Review comment:
   weird that `ActiveResourceManager#getCpuCores` returns double, though we need `CPUResource` which is in general returned from `getCpuCoresWithFallback`. we need int rewrapping only in yarn.
   This could also eliminate `TaskExecutorProcessSpecBuilder#withCpuCores(double cpuCores)`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "FAILURE",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/149269924) 
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/149298398) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   Hash:c330e78bc64b398d2ff483ab3f996db3958ec2fd Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/149269924 TriggerType:PUSH TriggerID:c330e78bc64b398d2ff483ab3f996db3958ec2fd
   Hash:453e643ea6705262b50dc0810fbd4ad3525b738d Status:SUCCESS URL:https://travis-ci.com/flink-ci/flink/builds/149298398 TriggerType:PUSH TriggerID:453e643ea6705262b50dc0810fbd4ad3525b738d
   Hash:453e643ea6705262b50dc0810fbd4ad3525b738d Status:SUCCESS URL:https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253 TriggerType:PUSH TriggerID:453e643ea6705262b50dc0810fbd4ad3525b738d
   -->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/149269924) 
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/149298398) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150291695",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5513",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8481273b59d64a27b53ee4dc87454998a6e0b895 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150291695) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5513) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383148341
 
 

 ##########
 File path: flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerTest.java
 ##########
 @@ -526,6 +527,45 @@ public void testOnStartContainerError() throws Exception {
 		}};
 	}
 
+	@Test
+	public void testGetCpuCoresCommonOption() throws Exception {
+		final Configuration configuration = new Configuration();
+		configuration.setDouble(TaskManagerOptions.CPU_CORES, 1.0);
 
 Review comment:
   It would be nice also to test rounding up to 1.0 and next greater integer for double values of `TaskManagerOptions.CPU_CORES`. Also, overrunning of `Integer.MAX_VALUE`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   Hash:c330e78bc64b398d2ff483ab3f996db3958ec2fd Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/149269924 TriggerType:PUSH TriggerID:c330e78bc64b398d2ff483ab3f996db3958ec2fd
   -->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/149269924) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383204017
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,7 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		double fallback = configuration.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU);
 
 Review comment:
   we should also deprecate this option

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383731182
 
 

 ##########
 File path: flink-mesos/src/main/java/org/apache/flink/mesos/runtime/clusterframework/MesosTaskManagerParameters.java
 ##########
 @@ -74,7 +74,7 @@
 		.withDescription(Description.builder().text("Disk space to assign to the Mesos workers in MB.").build());
 
 	public static final ConfigOption<Double> MESOS_RM_TASKS_CPUS =
-		key("mesos.resourcemanager.tasks.cpus")
+		key("mesos.resourcemanager.tasks.cpus").doubleType()
 
 Review comment:
   new line

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] xintongsong commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
xintongsong commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-590276879
 
 
   Thanks for the review, @wangyang0918 @azagrebin.
   Comments addressed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/149298398) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   Hash:c330e78bc64b398d2ff483ab3f996db3958ec2fd Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/149269924 TriggerType:PUSH TriggerID:c330e78bc64b398d2ff483ab3f996db3958ec2fd
   Hash:453e643ea6705262b50dc0810fbd4ad3525b738d Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:453e643ea6705262b50dc0810fbd4ad3525b738d
   -->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/149269924) 
   * 453e643ea6705262b50dc0810fbd4ad3525b738d UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   Hash:c330e78bc64b398d2ff483ab3f996db3958ec2fd Status:UNKNOWN URL:TBD TriggerType:PUSH TriggerID:c330e78bc64b398d2ff483ab3f996db3958ec2fd
   -->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/149298398) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253) 
   * 8481273b59d64a27b53ee4dc87454998a6e0b895 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383151412
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,7 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		double fallback = configuration.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU);
+		return TaskExecutorProcessUtils.getCpuCoresWithFallback(configuration, fallback).getValue().doubleValue();
 
 Review comment:
   I think this is difficult to unify among all active RMs, because we do operations over concrete number types in `getCpuCoresWithFallback` and yarn fallback option is Integer but others are double.
   We could have one more shortcut for double though for Mesos/K8s:
   ```
   public static CPUResource getCpuCoresWithFallback(final Configuration config, ConfigOption<Double> fallback)
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383740182
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,6 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		return TaskExecutorProcessUtils.getCpuCoresWithFallbackConfigOption(configuration, KubernetesConfigOptions.TASK_MANAGER_CPU);
 
 Review comment:
   True. It would be better to warp `CPUResource` inside `ActiveResourceManager#getCpuCores`.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   Hash:c330e78bc64b398d2ff483ab3f996db3958ec2fd Status:FAILURE URL:https://travis-ci.com/flink-ci/flink/builds/149269924 TriggerType:PUSH TriggerID:c330e78bc64b398d2ff483ab3f996db3958ec2fd
   Hash:453e643ea6705262b50dc0810fbd4ad3525b738d Status:PENDING URL:https://travis-ci.com/flink-ci/flink/builds/149298398 TriggerType:PUSH TriggerID:453e643ea6705262b50dc0810fbd4ad3525b738d
   -->
   ## CI report:
   
   * c330e78bc64b398d2ff483ab3f996db3958ec2fd Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/149269924) 
   * 453e643ea6705262b50dc0810fbd4ad3525b738d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/149298398) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] azagrebin closed pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
azagrebin closed pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#issuecomment-586973533
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149269924",
       "triggerID" : "c330e78bc64b398d2ff483ab3f996db3958ec2fd",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/149298398",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5253",
       "triggerID" : "453e643ea6705262b50dc0810fbd4ad3525b738d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150291695",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5513",
       "triggerID" : "8481273b59d64a27b53ee4dc87454998a6e0b895",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8481273b59d64a27b53ee4dc87454998a6e0b895 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150291695) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5513) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.

Posted by GitBox <gi...@apache.org>.
xintongsong commented on a change in pull request #11110: [FLINK-16111][k8s] Fix Kubernetes deployment not respecting 'taskmanager.cpu.cores'.
URL: https://github.com/apache/flink/pull/11110#discussion_r383208685
 
 

 ##########
 File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
 ##########
 @@ -338,6 +339,7 @@ protected FlinkKubeClient createFlinkKubeClient() {
 
 	@Override
 	protected double getCpuCores(Configuration configuration) {
-		return flinkConfig.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU, numSlotsPerTaskManager);
+		double fallback = configuration.getDouble(KubernetesConfigOptions.TASK_MANAGER_CPU);
 
 Review comment:
   Not sure about this.
   We have not expose `TaskManagerOptions#CPU_CORES` to the users. Its annotated `ExcludeFromDocumentation`.
   For the same reason, we have not deprecated the cpu config option for Yarn/Mesos.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services