Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-3150

Make YARN container invocation configurable

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: YARN
    • Labels:

      Description

      Currently, the JVM invocation call of YARN containers is hardcoded.

      With this change, I would like to make the call configurable, using a string such as
      "%java% %memopts% %jvmopts% ..."

      Also, we should respect the java.env.home if its set.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user rmetzger opened a pull request:

          https://github.com/apache/flink/pull/1446

          FLINK-3150 Make the YARN container start invocation configurable

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/rmetzger/flink flink3150

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/1446.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 #1446


          commit 3a482a3daeb7fed1cd89b27cd4f895b60b64980d
          Author: Robert Metzger <rmetzger@apache.org>
          Date: 2015-12-09T10:50:28Z

          FLINK-3150 Make the YARN container start invocation configurable


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user rmetzger opened a pull request: https://github.com/apache/flink/pull/1446 FLINK-3150 Make the YARN container start invocation configurable You can merge this pull request into a Git repository by running: $ git pull https://github.com/rmetzger/flink flink3150 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/1446.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 #1446 commit 3a482a3daeb7fed1cd89b27cd4f895b60b64980d Author: Robert Metzger <rmetzger@apache.org> Date: 2015-12-09T10:50:28Z FLINK-3150 Make the YARN container start invocation configurable
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on the pull request:

          https://github.com/apache/flink/pull/1446#issuecomment-176845053

          What's the state of this? Who do we need it? Is it still valid with the (planned) reworking of Yarn/Mesos integration?

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/1446#issuecomment-176845053 What's the state of this? Who do we need it? Is it still valid with the (planned) reworking of Yarn/Mesos integration?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the pull request:

          https://github.com/apache/flink/pull/1446#issuecomment-199858600

          I'm closing this PR and reopen it once the new resource management has been merged

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/1446#issuecomment-199858600 I'm closing this PR and reopen it once the new resource management has been merged
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger closed the pull request at:

          https://github.com/apache/flink/pull/1446

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger closed the pull request at: https://github.com/apache/flink/pull/1446
          Hide
          rmetzger Robert Metzger added a comment -

          I ran into this issue again.
          I'll try to open a PR for it soon. The code we need to change is the BootstrapTools.getTaskManagerShellCommand() method (we should unify this with the AbstractYarnClusterDescriptor.setupApplicationMasterContainer() invocation (like my previous PR).

          Show
          rmetzger Robert Metzger added a comment - I ran into this issue again. I'll try to open a PR for it soon. The code we need to change is the BootstrapTools.getTaskManagerShellCommand() method (we should unify this with the AbstractYarnClusterDescriptor.setupApplicationMasterContainer() invocation (like my previous PR).
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user NicoK opened a pull request:

          https://github.com/apache/flink/pull/3056

          FLINK-3150 make YARN container invocation configurable

          By using the `yarn.container-start-command-template` configuration parameter, the Flink start command can be altered/extended. By default, it is set to `"%java% %jvmmem% %jvmopts% %logging% %class% %args% %redirects%"` with

          • `java` = path to the Java executable
          • `jvmmem` = JVM memory limits and tweaks
          • `jvmopts` = misc options for the Java VM
          • `logging` = logging-related configuration settings
          • `class` = main class to execute
          • `args` = arguments for the main class
          • `redirects` = output redirects

          @rmetzger this is based on your previous pull request (https://github.com/apache/flink/pull/1446/files), can you have a look?

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/NicoK/flink FLINK-3150

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3056.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 #3056


          commit f36599778d5e28401d68bbdd39d0c55e2eee3d0b
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-02T15:17:09Z

          [hotfix] fix YARN TM krb5 conf only available when logging enabled

          commit f112d0c46d205623712fd8cda0c798152150dfb1
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-02T16:44:38Z

          FLINK-3150 add a test for BootstrapTools#getTaskManagerShellCommand

          commit deb04f971764a675b5e39a5609465f0179fa8d02
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-03T10:53:23Z

          FLINK-3150 add a test for AbstractYarnClusterDescriptor#setupApplicationMasterContainer

          commit a140f34ebe728c77f8afc051b3dda0066a252bc6
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-02T16:01:21Z

          FLINK-3150 make YARN container invocation configurable

          By using the `yarn.container-start-command-template` configuration parameter,
          the Flink start command can be altered/extended. By default, it is set to
          `"%java% %jvmmem% %jvmopts% %logging% %class% %args% %redirects%"` with

          • `java` = path to the Java executable
          • `jvmmem` = JVM memory limits and tweaks
          • `jvmopts` = misc options for the Java VM
          • `logging` = logging-related configuration settings
          • `class` = main class to execute
          • `args` = arguments for the main class
          • `redirects` = output redirects

          This is based on a previous pull request and commits by @rmetzger, i.e.
          https://github.com/apache/flink/pull/1446/files

          commit b17d563a626da17c82476a4b76fd62da325e4396
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-02T17:26:31Z

          FLINK-3150 fe-factor BootstrapToolsTest#testGetTaskManagerShellCommand

          -> reduce duplication of strings for easier test changes

          commit 6ff6724c2935956a0deea51286eb0930652a165f
          Author: Nico Kruber <nico@data-artisans.com>
          Date: 2017-01-02T17:27:33Z

          FLINK-3150 extend BootstrapToolsTest#testGetTaskManagerShellCommand

          This now also tests some variants changing the new
          `yarn.container-start-command-template` configuration parameter.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user NicoK opened a pull request: https://github.com/apache/flink/pull/3056 FLINK-3150 make YARN container invocation configurable By using the `yarn.container-start-command-template` configuration parameter, the Flink start command can be altered/extended. By default, it is set to `"%java% %jvmmem% %jvmopts% %logging% %class% %args% %redirects%"` with `java` = path to the Java executable `jvmmem` = JVM memory limits and tweaks `jvmopts` = misc options for the Java VM `logging` = logging-related configuration settings `class` = main class to execute `args` = arguments for the main class `redirects` = output redirects @rmetzger this is based on your previous pull request ( https://github.com/apache/flink/pull/1446/files ), can you have a look? You can merge this pull request into a Git repository by running: $ git pull https://github.com/NicoK/flink FLINK-3150 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3056.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 #3056 commit f36599778d5e28401d68bbdd39d0c55e2eee3d0b Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-02T15:17:09Z [hotfix] fix YARN TM krb5 conf only available when logging enabled commit f112d0c46d205623712fd8cda0c798152150dfb1 Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-02T16:44:38Z FLINK-3150 add a test for BootstrapTools#getTaskManagerShellCommand commit deb04f971764a675b5e39a5609465f0179fa8d02 Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-03T10:53:23Z FLINK-3150 add a test for AbstractYarnClusterDescriptor#setupApplicationMasterContainer commit a140f34ebe728c77f8afc051b3dda0066a252bc6 Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-02T16:01:21Z FLINK-3150 make YARN container invocation configurable By using the `yarn.container-start-command-template` configuration parameter, the Flink start command can be altered/extended. By default, it is set to `"%java% %jvmmem% %jvmopts% %logging% %class% %args% %redirects%"` with `java` = path to the Java executable `jvmmem` = JVM memory limits and tweaks `jvmopts` = misc options for the Java VM `logging` = logging-related configuration settings `class` = main class to execute `args` = arguments for the main class `redirects` = output redirects This is based on a previous pull request and commits by @rmetzger, i.e. https://github.com/apache/flink/pull/1446/files commit b17d563a626da17c82476a4b76fd62da325e4396 Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-02T17:26:31Z FLINK-3150 fe-factor BootstrapToolsTest#testGetTaskManagerShellCommand -> reduce duplication of strings for easier test changes commit 6ff6724c2935956a0deea51286eb0930652a165f Author: Nico Kruber <nico@data-artisans.com> Date: 2017-01-02T17:27:33Z FLINK-3150 extend BootstrapToolsTest#testGetTaskManagerShellCommand This now also tests some variants changing the new `yarn.container-start-command-template` configuration parameter.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3056#discussion_r94634138

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java —
          @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand(
          boolean hasKrb5,
          Class<?> mainClass) {
          — End diff –

          Did you consider using this method also for the JobManager / ApplicationMaster container invocation in `AbstractYarnClusterDescriptor.java` ? I'm not sure if my approach is the right one, but there is a lot of shared code between the two.

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/3056#discussion_r94634138 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java — @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand( boolean hasKrb5, Class<?> mainClass) { — End diff – Did you consider using this method also for the JobManager / ApplicationMaster container invocation in `AbstractYarnClusterDescriptor.java` ? I'm not sure if my approach is the right one, but there is a lot of shared code between the two.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

          https://github.com/apache/flink/pull/3056

          Thank you for looking into the issue. Before I'm merging it, I'd also like to test drive the change on a real cluster

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3056 Thank you for looking into the issue. Before I'm merging it, I'd also like to test drive the change on a real cluster
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user NicoK commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3056#discussion_r94798020

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java —
          @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand(
          boolean hasKrb5,
          Class<?> mainClass) {
          — End diff –

          There were some subtle differences in the memory setup (is that really intended?)), log and config directories and files which is why I did not generalise the setup.
          It may be done, however... question is whether it is easier to grasp afterwards. Or let's worry about this when there are three setups of this kind

          Show
          githubbot ASF GitHub Bot added a comment - Github user NicoK commented on a diff in the pull request: https://github.com/apache/flink/pull/3056#discussion_r94798020 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java — @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand( boolean hasKrb5, Class<?> mainClass) { — End diff – There were some subtle differences in the memory setup (is that really intended?)), log and config directories and files which is why I did not generalise the setup. It may be done, however... question is whether it is easier to grasp afterwards. Or let's worry about this when there are three setups of this kind
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3056#discussion_r94954355

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java —
          @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand(
          boolean hasKrb5,
          Class<?> mainClass) {
          — End diff –

          Okay, then lets leave it as is for now.

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/3056#discussion_r94954355 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java — @@ -347,43 +351,88 @@ public static String getTaskManagerShellCommand( boolean hasKrb5, Class<?> mainClass) { — End diff – Okay, then lets leave it as is for now.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user rmetzger commented on the issue:

          https://github.com/apache/flink/pull/3056

          +1 to merge. The cluster test was successful.

          Show
          githubbot ASF GitHub Bot added a comment - Github user rmetzger commented on the issue: https://github.com/apache/flink/pull/3056 +1 to merge. The cluster test was successful.
          Hide
          rmetzger Robert Metzger added a comment -

          Thanks a lot for implementing this.

          Merged in master: http://git-wip-us.apache.org/repos/asf/flink/commit/8f4139a4

          Show
          rmetzger Robert Metzger added a comment - Thanks a lot for implementing this. Merged in master: http://git-wip-us.apache.org/repos/asf/flink/commit/8f4139a4
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3056

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3056

            People

            • Assignee:
              NicoK Nico Kruber
              Reporter:
              rmetzger Robert Metzger
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development