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

Possible leakage of FileSystem object when YarnUtils.addDelegationTokens is called with different user

    Details

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

      Description

      The FileSystem.get caches instance for different UGI by default and the cache will never cleanup until process shutdown.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user chtyim opened a pull request:

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

          (TWILL-227) Disabling caching of FileSystem instance when getting delegation token

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

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

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

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


          commit b2ce61f9023770aab9efd180b8e662212784d137
          Author: Terence Yim <chtyim@apache.org>
          Date: 2017-03-27T23:37:07Z

          (TWILL-227) Disabling caching of FileSystem instance when getting delegation token


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user chtyim opened a pull request: https://github.com/apache/twill/pull/45 ( TWILL-227 ) Disabling caching of FileSystem instance when getting delegation token You can merge this pull request into a Git repository by running: $ git pull https://github.com/chtyim/twill feature/ TWILL-227 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/twill/pull/45.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 #45 commit b2ce61f9023770aab9efd180b8e662212784d137 Author: Terence Yim <chtyim@apache.org> Date: 2017-03-27T23:37:07Z ( TWILL-227 ) Disabling caching of FileSystem instance when getting delegation token
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user chtyim closed the pull request at:

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

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

          GitHub user chtyim opened a pull request:

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

          (TWILL-227) Disabling caching of FileSystem instance when getting delegation token

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

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

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

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


          commit b2ce61f9023770aab9efd180b8e662212784d137
          Author: Terence Yim <chtyim@apache.org>
          Date: 2017-03-27T23:37:07Z

          (TWILL-227) Disabling caching of FileSystem instance when getting delegation token


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user chtyim opened a pull request: https://github.com/apache/twill/pull/46 ( TWILL-227 ) Disabling caching of FileSystem instance when getting delegation token You can merge this pull request into a Git repository by running: $ git pull https://github.com/chtyim/twill feature/ TWILL-227 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/twill/pull/46.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 #46 commit b2ce61f9023770aab9efd180b8e662212784d137 Author: Terence Yim <chtyim@apache.org> Date: 2017-03-27T23:37:07Z ( TWILL-227 ) Disabling caching of FileSystem instance when getting delegation token
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user chtyim commented on the issue:

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

          This replaces #45 due to accidentally deleting the branch.

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on the issue: https://github.com/apache/twill/pull/46 This replaces #45 due to accidentally deleting the branch.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/46#discussion_r108490923

          — Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java —
          @@ -159,19 +159,19 @@ public static ApplicationId createApplicationId(long timestamp, int id)

          { return ImmutableList.of(); }
          • FileSystem fileSystem = getFileSystem(locationFactory, config);
            -
          • if (fileSystem == null) { - LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); - return ImmutableList.of(); - }

            + try (FileSystem fileSystem = getFileSystem(locationFactory)) {
            + if (fileSystem == null) {
            + LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory");

              • End diff –

          seems that this message should be logged inside getFileSystem()

          Show
          githubbot ASF GitHub Bot added a comment - Github user anew commented on a diff in the pull request: https://github.com/apache/twill/pull/46#discussion_r108490923 — Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java — @@ -159,19 +159,19 @@ public static ApplicationId createApplicationId(long timestamp, int id) { return ImmutableList.of(); } FileSystem fileSystem = getFileSystem(locationFactory, config); - if (fileSystem == null) { - LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); - return ImmutableList.of(); - } + try (FileSystem fileSystem = getFileSystem(locationFactory)) { + if (fileSystem == null) { + LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); End diff – seems that this message should be logged inside getFileSystem()
          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/46#discussion_r108542463

          — Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java —
          @@ -159,19 +159,19 @@ public static ApplicationId createApplicationId(long timestamp, int id)

          { return ImmutableList.of(); }
          • FileSystem fileSystem = getFileSystem(locationFactory, config);
            -
          • if (fileSystem == null) { - LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); - return ImmutableList.of(); - }

            + try (FileSystem fileSystem = getFileSystem(locationFactory)) {
            + if (fileSystem == null) {
            + LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory");

              • End diff –

          Updated

          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/46#discussion_r108542463 — Diff: twill-yarn/src/main/java/org/apache/twill/internal/yarn/YarnUtils.java — @@ -159,19 +159,19 @@ public static ApplicationId createApplicationId(long timestamp, int id) { return ImmutableList.of(); } FileSystem fileSystem = getFileSystem(locationFactory, config); - if (fileSystem == null) { - LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); - return ImmutableList.of(); - } + try (FileSystem fileSystem = getFileSystem(locationFactory)) { + if (fileSystem == null) { + LOG.warn("Unexpected: LocationFactory is not backed by FileContextLocationFactory"); End diff – Updated
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development