Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1, 2.7.5
    • Component/s: None
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      UGI#getGroups and its usage is inefficient. The list is unnecessarily converted to multiple collections.

      For every invocation, the List<String> from the group provider is converted into a LinkedHashSet<String> (to de-dup), back to a String[]. Then callers testing for group membership convert back to a List<String>. This should be done once to reduce allocations.

        Issue Links

          Activity

          Hide
          daryn Daryn Sharp added a comment -
          1. Changed group provider to cache the de-dupped list instead of the raw list.
          2. Added new UGI#getGroups that returns the aforementioned de-duped list
          3. Changed UGI#getPrimaryGroup to call UGI#getGroups to avoid an array copy
          4. Removed unnecessary synchronization of UGI#getGroups method. Required minor tweak to Groups#getGroups to be thread-safe. Already used elsewhere w/o synch, so this just makes it safe. Reduces contention with cached token->ugi instances.
          Show
          daryn Daryn Sharp added a comment - Changed group provider to cache the de-dupped list instead of the raw list. Added new UGI#getGroups that returns the aforementioned de-duped list Changed UGI#getPrimaryGroup to call UGI#getGroups to avoid an array copy Removed unnecessary synchronization of UGI#getGroups method. Required minor tweak to Groups#getGroups to be thread-safe. Already used elsewhere w/o synch, so this just makes it safe. Reduces contention with cached token->ugi instances.
          Hide
          daryn Daryn Sharp added a comment -

          Will be much more useful with hdfs counterpart.

          Show
          daryn Daryn Sharp added a comment - Will be much more useful with hdfs counterpart.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 34s trunk passed
          +1 compile 6m 41s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 53s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 19s trunk passed
          +1 javadoc 0m 45s trunk passed
          +1 mvninstall 0m 37s the patch passed
          +1 compile 6m 38s the patch passed
          +1 javac 6m 38s the patch passed
          -0 checkstyle 0m 28s hadoop-common-project/hadoop-common: The patch generated 2 new + 432 unchanged - 4 fixed = 434 total (was 436)
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 26s the patch passed
          +1 javadoc 0m 45s the patch passed
          -1 unit 16m 58s hadoop-common in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          46m 40s



          Reason Tests
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821047/HADOOP-13442.patch
          JIRA Issue HADOOP-13442
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b6a6ffff0f66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 95f2b98
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 34s trunk passed +1 compile 6m 41s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 19s trunk passed +1 javadoc 0m 45s trunk passed +1 mvninstall 0m 37s the patch passed +1 compile 6m 38s the patch passed +1 javac 6m 38s the patch passed -0 checkstyle 0m 28s hadoop-common-project/hadoop-common: The patch generated 2 new + 432 unchanged - 4 fixed = 434 total (was 436) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 26s the patch passed +1 javadoc 0m 45s the patch passed -1 unit 16m 58s hadoop-common in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 46m 40s Reason Tests Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821047/HADOOP-13442.patch JIRA Issue HADOOP-13442 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b6a6ffff0f66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 95f2b98 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10121/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          kihwal Kihwal Lee added a comment -

          +1 nice changes. The removal of synchronization looks safe after the changes to the static mapping.

          Show
          kihwal Kihwal Lee added a comment - +1 nice changes. The removal of synchronization looks safe after the changes to the static mapping.
          Hide
          kihwal Kihwal Lee added a comment -

          Committed to trunk, branch-2 and branch-2.8.

          Show
          kihwal Kihwal Lee added a comment - Committed to trunk, branch-2 and branch-2.8.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #10215 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10215/)
          HADOOP-13442. Optimize UGI group lookups. Contributed by Daryn Sharp. (kihwal: rev 94225152399e6e89fa7b4cff6d17d33e544329a3)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #10215 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10215/ ) HADOOP-13442 . Optimize UGI group lookups. Contributed by Daryn Sharp. (kihwal: rev 94225152399e6e89fa7b4cff6d17d33e544329a3) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFs.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
          Hide
          shv Konstantin Shvachko added a comment -

          Just committed along with broken tests HDFS-10738 and MAPREDUCE-6750 to branch-2.7. Thank you Daryn.
          Updating Fix Version for all three.

          Show
          shv Konstantin Shvachko added a comment - Just committed along with broken tests HDFS-10738 and MAPREDUCE-6750 to branch-2.7. Thank you Daryn. Updating Fix Version for all three.

            People

            • Assignee:
              daryn Daryn Sharp
              Reporter:
              daryn Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development