You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by WangTaoTheTonic <gi...@git.apache.org> on 2017/02/24 08:44:33 UTC
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
GitHub user WangTaoTheTonic opened a pull request:
https://github.com/apache/flink/pull/3408
[FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots and yarn.containers.vcores in YARN mode
Make sure taskmanager.numberOfTaskSlots and yarn.containers.vcores works in YARN mode. The priorities is: -s/-ys > yarn.containers.vcores > taskmanager.numberOfTaskSlots.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WangTaoTheTonic/flink FLINK-5903
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3408.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3408
----
commit 3515f9faf4b40bff7310a55b7094b52999525cbb
Author: WangTaoTheTonic <wa...@huawei.com>
Date: 2017-02-24T08:11:47Z
xxx
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3408
Changes look good to me. Thanks for your contribution @WangTaoTheTonic. Merging this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104836285
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
@tillrohrmann You mean YARN_VCORES is deprecated now? After checking code I found there're two places where we still use it: [here](https://github.com/apache/flink/blob/master/flink-yarn/src/main/java/org/apache/flink/yarn/AbstractYarnClusterDescriptor.java#L317) and [here](https://github.com/apache/flink/blob/master/flink-yarn/src/main/java/org/apache/flink/yarn/YarnFlinkResourceManager.java#L339), especially the latter one is used for container request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3408
If I'm not mistaken, then `yarn.containers.vcores` defines the number of vcores for a container. This, however, is not the same as the number of slots, since a slot can also have more than one vcore assigned.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104867661
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -537,7 +543,6 @@ public YarnClusterClient createCluster(
Preconditions.checkNotNull(userJarFiles, "User jar files should not be null.");
AbstractYarnClusterDescriptor yarnClusterDescriptor = createDescriptor(applicationName, cmdLine);
- yarnClusterDescriptor.setFlinkConfiguration(config);
--- End diff --
all right
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104855322
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
And in document, there still has introduction of YARN_VCORES.
```
yarn.containers.vcores The number of virtual cores (vcores) per YARN container. By default, the
number of vcores is set to the number of slots per TaskManager, if set, or to 1, otherwise.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
@tillrohrmann @StephanEwen
The code was changed and I've verified the functions, could you please review this and merge it if it's good to go?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104124552
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
I think the preference order should be `TASK_MANAGER_NUM_TASK_SLOTS` > `YARN_VCORES` for the slots.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104277608
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
@tillrohrmann I'm agree with you. We need make sense that YARN_VCORES is available in yarn mode, and yarn/mesos/standalone should have different configuration. And `YARN_VCORES` is the sum of `Resource#getVirtualCores` response from yarn client.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3408
Yes exactly @WangTaoTheTonic :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
@tillrohrmann
After changing code the test results(both session and single mode) is like:
| Configurations | #vcores of container(TM) | #slots of TM |
| ---------------------------------------- | ------------------------ | ------------ |
| -s/-ys 5, yarn.containers.vcores: 4, taskmanager.numberOfTaskSlots: 3 | 4 | 5 |
| yarn.containers.vcores: 4, taskmanager.numberOfTaskSlots: 3 | 4 | 3 |
| yarn.containers.vcores: 4 | 4 | 1 |
| -s/-ys 5, taskmanager.numberOfTaskSlots: 3 | 5 | 5 |
| taskmanager.numberOfTaskSlots: 3 | 3 | 3 |
| Nothing to specify, all use default | 1 | 1 |
Please check :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104862066
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -537,7 +543,6 @@ public YarnClusterClient createCluster(
Preconditions.checkNotNull(userJarFiles, "User jar files should not be null.");
AbstractYarnClusterDescriptor yarnClusterDescriptor = createDescriptor(applicationName, cmdLine);
- yarnClusterDescriptor.setFlinkConfiguration(config);
--- End diff --
This might be the case right now, but it could also change in the future. The signature of the `createCluster` method contains the configuration parameter and, thus, every user will think he can pass a configuration for the creation. We should keep that way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
I move the initiallization of this config to constructor of cluster descripter and restore the deleted configuration setting.
Please check if we are good with the usage of `YARN_VCORES`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104837998
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -537,7 +543,6 @@ public YarnClusterClient createCluster(
Preconditions.checkNotNull(userJarFiles, "User jar files should not be null.");
AbstractYarnClusterDescriptor yarnClusterDescriptor = createDescriptor(applicationName, cmdLine);
- yarnClusterDescriptor.setFlinkConfiguration(config);
--- End diff --
@tillrohrmann Under what condition this configuration will contains values added programmatically? I've checked the codes and only found this config is initiallized from config file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r108417249
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/AbstractYarnClusterDescriptor.java ---
@@ -164,6 +164,12 @@ public AbstractYarnClusterDescriptor() {
throw new RuntimeException("Unable to locate configuration file in " + confFile);
}
flinkConfigurationPath = new Path(confFile.getAbsolutePath());
+
+ if (flinkConfiguration.containsKey(ConfigConstants.YARN_VCORES)) {
+ slots = flinkConfiguration.getInteger(ConfigConstants.YARN_VCORES, -1);
+ } else if (flinkConfiguration.containsKey(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS)) {
+ slots = flinkConfiguration.getInteger(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS, -1);
+ }
--- End diff --
I think we should make the behaviour consistent with Mesos and standalone where we set `slots = flinkConfiguration.getInteger(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS, 1)`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
I'm sorry about the commit message :(
next time I'll format it, as it's better not to squash.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3408
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
@tillrohrmann I still cannot get your point entirely. don't this three configs(`-s/-ys`, `yarn.containers.vcores` and `taskmanager.numberOfTaskSlots` mean same thing? Do they have difference in usage except priority?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
All right. That makes sense. Let me rephrase that and please check if we are in same channel:
1. slots of taskmanager is decided by `-s/-ys` and `taskmanager.numberOfTaskSlots`, the former has higher priority; and
2. the vcores of yarn container is decided by `yarn.containers.vcores`, which will use values of `-s/-ys` or `taskmanager.numberOfTaskSlots` if user doesn't set `yarn.containers.vcores` explicitly.
Is that right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104125371
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -537,7 +543,6 @@ public YarnClusterClient createCluster(
Preconditions.checkNotNull(userJarFiles, "User jar files should not be null.");
AbstractYarnClusterDescriptor yarnClusterDescriptor = createDescriptor(applicationName, cmdLine);
- yarnClusterDescriptor.setFlinkConfiguration(config);
--- End diff --
You cannot simply ignore the `configuration` which has been given to `createCluster`. What if it contains configuration values which were added programmatically?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104861770
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
No YARN_VCORES is not deprecated. But I'm not sure whether we should use this value as a default value for the number of task manager slots.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3408
Another remark concerning the commit message. In the Flink community it's common to format the commit message the following way:
```
[FLINK-XXXX] [component tag] Commit message title
Commit message body
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104125523
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -537,7 +543,6 @@ public YarnClusterClient createCluster(
Preconditions.checkNotNull(userJarFiles, "User jar files should not be null.");
AbstractYarnClusterDescriptor yarnClusterDescriptor = createDescriptor(applicationName, cmdLine);
- yarnClusterDescriptor.setFlinkConfiguration(config);
--- End diff --
You should give this configuration to the `createDescriptor` method where it is used to instantiate the `YarnClusterDescriptor`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104128524
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
Actually, I'm not so sure whether whether we should use the `YARN_VCORES` value as a fallback value for the number of task manager slots. We had this in the past but we changed it so that the default behaviour is if the number of slots is not specified, then it is `1`. Changing this here for Yarn would make the behaviour inconsistent wrt Mesos, for example. From a user's perspective the old behaviour is imo more predictable. However, we should definitely respect the configuration file settings for the task manager slots.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r104862308
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -317,6 +319,10 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(SLOTS.getOpt())) {
int slots = Integer.valueOf(cmd.getOptionValue(SLOTS.getOpt()));
yarnClusterDescriptor.setTaskManagerSlots(slots);
+ } else if (config.containsKey(ConfigConstants.YARN_VCORES)) {
--- End diff --
the document says YARN_VCORE > slotsOfTaskManager > default value(1) shen they are set in file.
the parameters in command line will be used before those in config file, in some way it is a common sense :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTaskSlots a...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3408
ping @tillrohrmann
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3408: [FLINK-5903][YARN]respect taskmanager.numberOfTask...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3408#discussion_r108417410
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/YarnFlinkResourceManager.java ---
@@ -335,8 +335,7 @@ protected void requestNewWorkers(int numWorkers) {
Priority priority = Priority.newInstance(0);
// Resource requirements for worker containers
- int taskManagerSlots = taskManagerParameters.numSlots();
- int vcores = config.getInteger(ConfigConstants.YARN_VCORES, Math.max(taskManagerSlots, 1));
+ int vcores = Math.max(taskManagerParameters.numSlots(), 1);
--- End diff --
I think we should still keep the possibility open to configure the `vcores` explicitly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---