Hadoop Common
  1. Hadoop Common
  2. HADOOP-10401

ShellBasedUnixGroupsMapping#getGroups does not always return primary group first

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.0
    • Fix Version/s: 2.5.0
    • Component/s: util
    • Labels:
      None
    • Target Version/s:

      Description

      ShellBasedUnixGroupsMapping#getGroups does not always return the primary group first. It should do this so that clients who expect it don't get the wrong result. We should also document that the primary group is returned first in the API. Note that JniBasedUnixGroupsMapping does return the primary group first.

      1. HADOOP-10401.patch
        3 kB
        Akira AJISAKA
      2. HADOOP-10401.2.patch
        3 kB
        Akira AJISAKA

        Issue Links

          Activity

          Colin Patrick McCabe created issue -
          Colin Patrick McCabe made changes -
          Field Original Value New Value
          Link This issue is related to HADOOP-10087 [ HADOOP-10087 ]
          Akira AJISAKA made changes -
          Assignee Akira AJISAKA [ ajisakaa ]
          Hide
          Akira AJISAKA added a comment -

          Attaching a sample patch for unix.

          Show
          Akira AJISAKA added a comment - Attaching a sample patch for unix.
          Akira AJISAKA made changes -
          Attachment HADOOP-10401.patch [ 12635226 ]
          Hide
          Colin Patrick McCabe added a comment -

          Hi Akira,

          Thanks for looking at this!

          My concern with the first patch is that you are now spawning two external processes instead of one. It seems like this could regress performance somewhat. There can be a large overhead to spawning an external process from the JVM due to the need to copy page tables during fork().

          I think maybe we should consider executing a shell, which then executes both "id -gn" and "id -Gn". Perhaps something like 'sh -c id -gn ; id -Gn'

          Show
          Colin Patrick McCabe added a comment - Hi Akira, Thanks for looking at this! My concern with the first patch is that you are now spawning two external processes instead of one. It seems like this could regress performance somewhat. There can be a large overhead to spawning an external process from the JVM due to the need to copy page tables during fork(). I think maybe we should consider executing a shell, which then executes both "id -gn" and "id -Gn". Perhaps something like 'sh -c id -gn ; id -Gn'
          Hide
          Chris Nauroth added a comment -

          I suggest && instead of ; for separating commands. We wouldn't want to mask a failure in the first command if the second command is successful. (That kind of failure should be a really really rare thing of course, but let's try to catch it just in case.)

          Show
          Chris Nauroth added a comment - I suggest && instead of ; for separating commands. We wouldn't want to mask a failure in the first command if the second command is successful. (That kind of failure should be a really really rare thing of course, but let's try to catch it just in case.)
          Hide
          Akira AJISAKA added a comment -

          Thanks for the comments, Colin, Chris. I'll update the patch.

          Show
          Akira AJISAKA added a comment - Thanks for the comments, Colin, Chris. I'll update the patch.
          Akira AJISAKA made changes -
          Attachment HADOOP-10401.2.patch [ 12635370 ]
          Akira AJISAKA made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12635370/HADOOP-10401.2.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3679//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3679//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12635370/HADOOP-10401.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3679//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3679//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -

          +1. thanks, Akira

          Show
          Colin Patrick McCabe added a comment - +1. thanks, Akira
          Colin Patrick McCabe made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Target Version/s 2.5.0 [ 12326263 ]
          Fix Version/s 2.5.0 [ 12326263 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #5605 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5605/)
          HADOOP-10401. ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5605 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5605/ ) HADOOP-10401 . ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #562 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/562/)
          HADOOP-10401. ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #562 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/562/ ) HADOOP-10401 . ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #1754 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1754/)
          HADOOP-10401. ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1754 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1754/ ) HADOOP-10401 . ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1780 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1780/)
          HADOOP-10401. ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714)

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1780 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1780/ ) HADOOP-10401 . ShellBasedUnixGroupsMapping#getGroups does not always return primary group first (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1594714 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          Karthik Kambatla (Inactive) made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          7d 15h 54m 1 Akira AJISAKA 18/Mar/14 19:16
          Patch Available Patch Available Resolved Resolved
          57d 1h 50m 1 Colin Patrick McCabe 14/May/14 22:07
          Resolved Resolved Closed Closed
          92d 8h 32m 1 Karthik Kambatla (Inactive) 15/Aug/14 06:39

            People

            • Assignee:
              Akira AJISAKA
              Reporter:
              Colin Patrick McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development