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

[yarn] Support downloading additional jars from non-HDFS paths

    Details

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

      Description

      Currently the YarnResourceManager and YarnApplicationMasterRunner always register the additional jars using the YARN filesystem object. This is problematic as the paths might require another filesystem.

      To support localizing from non-HDFS paths (e.g., s3, http or viewfs), the cleaner approach is to get the filesystem object from the path.

        Issue Links

          Activity

          Hide
          StephanEwen Stephan Ewen added a comment -

          Fixed in 186b12309b540f82a055be28f3f005dce4b8cf46

          Thank you for the contribution!

          Show
          StephanEwen Stephan Ewen added a comment - Fixed in 186b12309b540f82a055be28f3f005dce4b8cf46 Thank you for the contribution!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          Github user StephanEwen commented on the issue:

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

          The change looks good to me, +1 from my side

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3202 The change looks good to me, +1 from my side
          Hide
          wheat9 Haohui Mai added a comment -

          Stephan Ewen can you please take another look?

          Show
          wheat9 Haohui Mai added a comment - Stephan Ewen can you please take another look?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user haohui commented on the issue:

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

          Thanks for the reviews.

          It's more about readability of the code itself instead of testing and visibility. The two functions are fairly long and basically identical. Therefore I refactor the code a little bit to extract the common functions out into the Util class.

          To facilitate reviews I separate the changes into two commits. The first commit contains the changes, and the second commit does the refactoring.

          @StephanEwen please take another look.

          Show
          githubbot ASF GitHub Bot added a comment - Github user haohui commented on the issue: https://github.com/apache/flink/pull/3202 Thanks for the reviews. It's more about readability of the code itself instead of testing and visibility. The two functions are fairly long and basically identical. Therefore I refactor the code a little bit to extract the common functions out into the Util class. To facilitate reviews I separate the changes into two commits. The first commit contains the changes, and the second commit does the refactoring. @StephanEwen please take another look.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

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

          @haohui I think it would be nice to have this change without moving the function to a different class. Then the diffs would be easier to read (check what part of the functionality was actually changed).

          If this is mainly about testing and visibility, then you can increase the visibility of the method from `private` to `package-private`, add a `@VisibleForTesting` annotation and place the test in the same package.

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3202 @haohui I think it would be nice to have this change without moving the function to a different class. Then the diffs would be easier to read (check what part of the functionality was actually changed). If this is mainly about testing and visibility, then you can increase the visibility of the method from `private` to `package-private`, add a `@VisibleForTesting` annotation and place the test in the same package.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user haohui opened a pull request:

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

          FLINK-5631 [yarn] Support downloading additional jars from non-HDFS…

          This PR enables Flink to support downloading additional jars from non-HDFS paths (e.g., S3, viewfs). More details are available in the jira.

          There are two changes in this PR:

          • Refactoring the code for localization into a common function
          • Use `Path.getFileSystem()` to obtain the FileSystem object instead of always using the default FileSystem constructed using YARN config.

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

          $ git pull https://github.com/haohui/flink FLINK-5631

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

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


          commit f7d24c5a0559ca71cd49962c9dc0b4ab81997783
          Author: Haohui Mai <wheat9@apache.org>
          Date: 2017-01-25T02:00:08Z

          FLINK-5631 [yarn] Support downloading additional jars from non-HDFS paths.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user haohui opened a pull request: https://github.com/apache/flink/pull/3202 FLINK-5631 [yarn] Support downloading additional jars from non-HDFS… This PR enables Flink to support downloading additional jars from non-HDFS paths (e.g., S3, viewfs). More details are available in the jira. There are two changes in this PR: Refactoring the code for localization into a common function Use `Path.getFileSystem()` to obtain the FileSystem object instead of always using the default FileSystem constructed using YARN config. You can merge this pull request into a Git repository by running: $ git pull https://github.com/haohui/flink FLINK-5631 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3202.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 #3202 commit f7d24c5a0559ca71cd49962c9dc0b4ab81997783 Author: Haohui Mai <wheat9@apache.org> Date: 2017-01-25T02:00:08Z FLINK-5631 [yarn] Support downloading additional jars from non-HDFS paths.

            People

            • Assignee:
              wheat9 Haohui Mai
              Reporter:
              wheat9 Haohui Mai
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development