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/25 06:45:17 UTC
[GitHub] flink pull request #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
GitHub user WangTaoTheTonic opened a pull request:
https://github.com/apache/flink/pull/3414
[FLINK-5904][YARN]make jobmanager.heap.mb and taskmanager.heap.mb work in YARN mode
I'm making these two configuration items same with "-yjm""-ytm" in yarn session and "-jm""-tm" in single job. Looks like it might get some misunderstanding.
We don't have a proper config item here. What's better idea do you suggest?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WangTaoTheTonic/flink FLINK-5904
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3414.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 #3414
----
commit b21d27ea99ffa613a8eb9c00a4f977b619b12418
Author: WangTaoTheTonic <wa...@huawei.com>
Date: 2017-02-25T04:19:43Z
make jobmanager.heap.mb and taskmanager.heap.mb work in YARN mode
----
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104853211
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -306,12 +308,16 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(JM_MEMORY.getOpt())) {
int jmMemory = Integer.valueOf(cmd.getOptionValue(JM_MEMORY.getOpt()));
yarnClusterDescriptor.setJobManagerMemory(jmMemory);
+ } else if (config.containsKey(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY)) {
+ yarnClusterDescriptor.setJobManagerMemory(config.getInteger(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY, ConfigConstants.DEFAULT_JOB_MANAGER_HEAP_MEMORY));
--- End diff --
do you mean we should only create `jobManagerMemoryMb` object but not init it with value?
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3414
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104174086
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -306,12 +308,16 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(JM_MEMORY.getOpt())) {
int jmMemory = Integer.valueOf(cmd.getOptionValue(JM_MEMORY.getOpt()));
yarnClusterDescriptor.setJobManagerMemory(jmMemory);
+ } else if (config.containsKey(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY)) {
+ yarnClusterDescriptor.setJobManagerMemory(config.getInteger(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY, ConfigConstants.DEFAULT_JOB_MANAGER_HEAP_MEMORY));
--- End diff --
I'm wondering whether the `yarnClusterDescriptor` should not have been automatically instantiated with this value if it gets the configuration as a constructor parameter.
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104854352
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/ConfigConstants.java ---
@@ -110,7 +110,12 @@
public static final String EXECUTION_RETRY_DELAY_KEY = "execution-retries.delay";
// -------------------------------- Runtime -------------------------------
-
+
+ /**
+ * JVM heap size (in megabytes) for the JobManager
+ */
+ public static final String JOB_MANAGER_HEAP_MEMORY_KEY = "jobmanager.heap.mb";
--- End diff --
Nice. I've changed :)
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and taskmanager...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3414
Thanks. I've resolved conflicts. enjoy :)
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104862218
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -306,12 +308,16 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(JM_MEMORY.getOpt())) {
int jmMemory = Integer.valueOf(cmd.getOptionValue(JM_MEMORY.getOpt()));
yarnClusterDescriptor.setJobManagerMemory(jmMemory);
+ } else if (config.containsKey(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY)) {
+ yarnClusterDescriptor.setJobManagerMemory(config.getInteger(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY, ConfigConstants.DEFAULT_JOB_MANAGER_HEAP_MEMORY));
--- End diff --
no, but initialize the value in the constructor of 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 issue #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and taskmanager...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3414
Thanks for your contribution @WangTaoTheTonic. LGTM. Merging the 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 issue #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and taskmanager...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the issue:
https://github.com/apache/flink/pull/3414
@tillrohrmann I've changed per comments. Mind reviewing again? Thanks :)
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104862993
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java ---
@@ -306,12 +308,16 @@ public AbstractYarnClusterDescriptor createDescriptor(String defaultApplicationN
if (cmd.hasOption(JM_MEMORY.getOpt())) {
int jmMemory = Integer.valueOf(cmd.getOptionValue(JM_MEMORY.getOpt()));
yarnClusterDescriptor.setJobManagerMemory(jmMemory);
+ } else if (config.containsKey(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY)) {
+ yarnClusterDescriptor.setJobManagerMemory(config.getInteger(ConfigConstants.JOB_MANAGER_HEAP_MEMORY_KEY, ConfigConstants.DEFAULT_JOB_MANAGER_HEAP_MEMORY));
--- End diff --
That's a good idea. we don't need to set it explicitly here if we init it in constructor :)
---
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 #3414: [FLINK-5904][YARN]make jobmanager.heap.mb and task...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/3414#discussion_r104174440
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/ConfigConstants.java ---
@@ -110,7 +110,12 @@
public static final String EXECUTION_RETRY_DELAY_KEY = "execution-retries.delay";
// -------------------------------- Runtime -------------------------------
-
+
+ /**
+ * JVM heap size (in megabytes) for the JobManager
+ */
+ public static final String JOB_MANAGER_HEAP_MEMORY_KEY = "jobmanager.heap.mb";
--- End diff --
Can we introduce these values as a `ConfigOption` instead of an entry to `ConfigConstants`? I think it might go to `TaskManagerOptions` and to `JobManagerOptions`.
---
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.
---