Uploaded image for project: 'Apache Twill'
  1. Apache Twill
  2. TWILL-225

Allow using different configurations per application submission

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: None
    • Labels:
      None

      Description

      Currently there are couple configurations that can be provided via the hadoop Configuration object to the YarnTwillRunnerService. However, those configurations are global (same for all app launched through the same TwillRunnerService). It would be better if the TwillPreparer exposes method to alter the configuration for a given app submission.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user chtyim opened a pull request:

          https://github.com/apache/twill/pull/39

          (TWILL-225) Refactor TwillPreprer to allow configurations affecting a…

          …pplication behavior overridable per TwillPreprer

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

          $ git pull https://github.com/chtyim/twill feature/TWILL-225

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

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


          commit babc2e1b6598319956b4333d3c3ef2ca3400a13c
          Author: Terence Yim <chtyim@apache.org>
          Date: 2017-03-19T01:31:57Z

          (TWILL-225) Refactor TwillPreprer to allow configurations affecting application behavior overridable per TwillPreprer


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user chtyim opened a pull request: https://github.com/apache/twill/pull/39 ( TWILL-225 ) Refactor TwillPreprer to allow configurations affecting a… …pplication behavior overridable per TwillPreprer You can merge this pull request into a Git repository by running: $ git pull https://github.com/chtyim/twill feature/ TWILL-225 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/twill/pull/39.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 #39 commit babc2e1b6598319956b4333d3c3ef2ca3400a13c Author: Terence Yim <chtyim@apache.org> Date: 2017-03-19T01:31:57Z ( TWILL-225 ) Refactor TwillPreprer to allow configurations affecting application behavior overridable per TwillPreprer
          Hide
          yufeldman Yuliya Feldman added a comment -

          Terence Yim Great idea, otherwise it has to be pretty much TwillRunnerService per "run" with any modification to Configuration object

          Show
          yufeldman Yuliya Feldman added a comment - Terence Yim Great idea, otherwise it has to be pretty much TwillRunnerService per "run" with any modification to Configuration object
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/39#discussion_r107004427

          — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java —
          @@ -155,25 +153,16 @@ public String apply(Class<?> cls) {
          private ClassAcceptor classAcceptor;
          private final Map<String, Integer> maxRetries = Maps.newHashMap();

          • YarnTwillPreparer(YarnConfiguration yarnConfig, TwillSpecification twillSpec, RunId runId,
          • YarnAppClient yarnAppClient, String zkConnectString, Location appLocation, Set<URL> twillClassPaths,
            + YarnTwillPreparer(Configuration config, TwillSpecification twillSpec, RunId runId,
              • End diff –

          Any particular reason to switch to Configuration from YarnConfiguration?

          Show
          githubbot ASF GitHub Bot added a comment - Github user yufeldman commented on a diff in the pull request: https://github.com/apache/twill/pull/39#discussion_r107004427 — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java — @@ -155,25 +153,16 @@ public String apply(Class<?> cls) { private ClassAcceptor classAcceptor; private final Map<String, Integer> maxRetries = Maps.newHashMap(); YarnTwillPreparer(YarnConfiguration yarnConfig, TwillSpecification twillSpec, RunId runId, YarnAppClient yarnAppClient, String zkConnectString, Location appLocation, Set<URL> twillClassPaths, + YarnTwillPreparer(Configuration config, TwillSpecification twillSpec, RunId runId, End diff – Any particular reason to switch to Configuration from YarnConfiguration?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/39#discussion_r107007777

          — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java —
          @@ -155,25 +153,16 @@ public String apply(Class<?> cls) {
          private ClassAcceptor classAcceptor;
          private final Map<String, Integer> maxRetries = Maps.newHashMap();

          • YarnTwillPreparer(YarnConfiguration yarnConfig, TwillSpecification twillSpec, RunId runId,
          • YarnAppClient yarnAppClient, String zkConnectString, Location appLocation, Set<URL> twillClassPaths,
            + YarnTwillPreparer(Configuration config, TwillSpecification twillSpec, RunId runId,
              • End diff –

          It's more generic. We actually not using any specific method from `YarnConfiguration` in this class.

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/39#discussion_r107007777 — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java — @@ -155,25 +153,16 @@ public String apply(Class<?> cls) { private ClassAcceptor classAcceptor; private final Map<String, Integer> maxRetries = Maps.newHashMap(); YarnTwillPreparer(YarnConfiguration yarnConfig, TwillSpecification twillSpec, RunId runId, YarnAppClient yarnAppClient, String zkConnectString, Location appLocation, Set<URL> twillClassPaths, + YarnTwillPreparer(Configuration config, TwillSpecification twillSpec, RunId runId, End diff – It's more generic. We actually not using any specific method from `YarnConfiguration` in this class.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/39#discussion_r107262119

          — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java —
          @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL
          this.logLevels.put(runnableName, newLevels);
          }

          + /**
          + * Creates an

          {@link Credentials} by copying the {@link Credentials}

          of the current user.
          + */
          private Credentials createCredentials() {
          Credentials credentials = new Credentials();

          try

          { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + }

          catch (IOException e) {
          — End diff –

          shouldn't this be fatal?

          Show
          githubbot ASF GitHub Bot added a comment - Github user anwar6953 commented on a diff in the pull request: https://github.com/apache/twill/pull/39#discussion_r107262119 — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java — @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL this.logLevels.put(runnableName, newLevels); } + /** + * Creates an {@link Credentials} by copying the {@link Credentials} of the current user. + */ private Credentials createCredentials() { Credentials credentials = new Credentials(); try { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + } catch (IOException e) { — End diff – shouldn't this be fatal?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/39#discussion_r107264780

          — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java —
          @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL
          this.logLevels.put(runnableName, newLevels);
          }

          + /**
          + * Creates an

          {@link Credentials} by copying the {@link Credentials}

          of the current user.
          + */
          private Credentials createCredentials() {
          Credentials credentials = new Credentials();

          try

          { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + }

          catch (IOException e) {
          — End diff –

          I don't think so. Even if this call fail, if the caller is able to acquire required credentials in his own way and add them via the `addSecureStore` method, then the app can still be submitted and executed correctly.

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/39#discussion_r107264780 — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java — @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL this.logLevels.put(runnableName, newLevels); } + /** + * Creates an {@link Credentials} by copying the {@link Credentials} of the current user. + */ private Credentials createCredentials() { Credentials credentials = new Credentials(); try { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + } catch (IOException e) { — End diff – I don't think so. Even if this call fail, if the caller is able to acquire required credentials in his own way and add them via the `addSecureStore` method, then the app can still be submitted and executed correctly.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user anwar6953 commented on the issue:

          https://github.com/apache/twill/pull/39

          LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user anwar6953 commented on the issue: https://github.com/apache/twill/pull/39 LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/39#discussion_r107265236

          — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java —
          @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL
          this.logLevels.put(runnableName, newLevels);
          }

          + /**
          + * Creates an

          {@link Credentials} by copying the {@link Credentials}

          of the current user.
          + */
          private Credentials createCredentials() {
          Credentials credentials = new Credentials();

          try

          { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + }

          catch (IOException e) {
          — End diff –

          I guess app submission would fail if caller doesn't acquire required credentials in its own way anyways.

          Show
          githubbot ASF GitHub Bot added a comment - Github user anwar6953 commented on a diff in the pull request: https://github.com/apache/twill/pull/39#discussion_r107265236 — Diff: twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java — @@ -455,21 +475,40 @@ private void saveLogLevels(String runnableName, Map<String, LogEntry.Level> logL this.logLevels.put(runnableName, newLevels); } + /** + * Creates an {@link Credentials} by copying the {@link Credentials} of the current user. + */ private Credentials createCredentials() { Credentials credentials = new Credentials(); try { credentials.addAll(UserGroupInformation.getCurrentUser().getCredentials()); + } catch (IOException e) { — End diff – I guess app submission would fail if caller doesn't acquire required credentials in its own way anyways.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/twill/pull/39

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

            People

            • Assignee:
              chtyim Terence Yim
              Reporter:
              chtyim Terence Yim
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development