Hadoop Common
  1. Hadoop Common
  2. HADOOP-4727

Groups do not work for fuse-dfs out of the box on 0.19.0

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.19.0
    • Fix Version/s: 0.19.1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The groups functionality of fuse-dfs did not work for me in Hadoop 0.19.0. Everything shows up as group nobody.

      1. hadoop-4727.patch
        0.5 kB
        Brian Bockelman
      2. HADOOP-4727_20.txt
        0.5 kB
        Pete Wyckoff

        Activity

        Hide
        Brian Bockelman added a comment -

        Patch is trivial. Logic was incorrect.

        This might be a critical issue for some users.

        Show
        Brian Bockelman added a comment - Patch is trivial. Logic was incorrect. This might be a critical issue for some users.
        Hide
        Brian Bockelman added a comment -

        Submitting to Hudson. No unit test available... I'll let Pete look into this one; certainly it should be covered.

        Show
        Brian Bockelman added a comment - Submitting to Hudson. No unit test available... I'll let Pete look into this one; certainly it should be covered.
        Hide
        Pete Wyckoff added a comment -

        good catch!

        i don't think it is covered. probably need to augment testWrites to do a getFileStatus and check the the created file has the right group.

        Show
        Pete Wyckoff added a comment - good catch! i don't think it is covered. probably need to augment testWrites to do a getFileStatus and check the the created file has the right group.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12394700/hadoop-4727.patch
        against trunk revision 720930.

        +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 tests are needed for this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

        -1 core tests. The patch failed core unit tests.

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/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/12394700/hadoop-4727.patch against trunk revision 720930. +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 tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3659/console This message is automatically generated.
        Hide
        Pete Wyckoff added a comment -

        Hi Brian,

        I think you can unit test this by adding the following to the testWrites method in src/test/TestFuseDFS.java

              Runtime r = Runtime.getRuntime();
              String cmd = "stat -c %G " + mpoint + "/hello.txt";
              Process p = r.exec(cmd);
              p.waitFor();
              InputStream i = p.getInputStream();
              byte buf[] = new byte[i.available()];
              length = i.read(buf);
        
              String res = new String(buf,0, length-1);
              
              if(res.equals("root")) {
                assertEquals(foo.getGroup(), "supergroup");
              } else {
                assertEquals(foo.getGroup(), res);
              }
        
        
        Show
        Pete Wyckoff added a comment - Hi Brian, I think you can unit test this by adding the following to the testWrites method in src/test/TestFuseDFS.java Runtime r = Runtime .getRuntime(); String cmd = "stat -c %G " + mpoint + "/hello.txt" ; Process p = r.exec(cmd); p.waitFor(); InputStream i = p.getInputStream(); byte buf[] = new byte [i.available()]; length = i.read(buf); String res = new String (buf,0, length-1); if (res.equals( "root" )) { assertEquals(foo.getGroup(), "supergroup" ); } else { assertEquals(foo.getGroup(), res); }
        Hide
        Pete Wyckoff added a comment -

        forgot to mention - i think you also need to special case this supergroup entity of hdfs in fuse_dfs.c

          if (info->mGroup != NULL) {                                                                                                                                                                                
            //                                                                                                                                                                                                       
            // Critical section - protect from concurrent calls in different threads since                                                                                                                           
            // the struct below is static.                                                                                                                                                                           
            // (no returns until end)                                                                                                                                                                                
            //                                                                                                                                                                                                       
                                                                                                                                                                                                                     
            pthread_mutex_lock(&groupstruct_mutex);                                                                                                                                                                  
                                                                                                                                                                                                                     
            struct group *grp = getgrnam(info->mGroup);                                                                                                                                                              
            group_id = grp == NULL ? default_id : grp->gr_gid;                                                                                                                                                       
                                                                                                                                                                                                                     
            if (grp == NULL && strcmp(info->mGroup, "supergroup") == 0) {                                                                                                                                  
              group_id = 0; // i.e., root                                                                                                                                                                            
            }                                                                                                                                                                                                        
                                                                                                                                                                                                                     
            //                                                                                                                                                                                                       
            // End critical section                                                                                                                                                                                  
            //                                                                                                                                                                                                       
            pthread_mutex_unlock(&groupstruct_mutex);                                                                                                                                                                
                                                                                                                                                                                                                     
          }                  
        
        Show
        Pete Wyckoff added a comment - forgot to mention - i think you also need to special case this supergroup entity of hdfs in fuse_dfs.c if (info->mGroup != NULL) { // // Critical section - protect from concurrent calls in different threads since // the struct below is static . // (no returns until end) // pthread_mutex_lock(&groupstruct_mutex); struct group *grp = getgrnam(info->mGroup); group_id = grp == NULL ? default_id : grp->gr_gid; if (grp == NULL && strcmp(info->mGroup, "supergroup" ) == 0) { group_id = 0; // i.e., root } // // End critical section // pthread_mutex_unlock(&groupstruct_mutex); }
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > i think you also need to special case this supergroup entity of hdfs in fuse_dfs.c

        The user domain in HDFS and the user domain in local host are different. Being a superuser of HDFS does not automatically imply a superuser of the local host, and vice versa. So, I think we don't need this.

        Show
        Tsz Wo Nicholas Sze added a comment - > i think you also need to special case this supergroup entity of hdfs in fuse_dfs.c The user domain in HDFS and the user domain in local host are different. Being a superuser of HDFS does not automatically imply a superuser of the local host, and vice versa. So, I think we don't need this.
        Hide
        Pete Wyckoff added a comment -

        What should fuse return when the group is supergroup then? the current code will end of returning 'nobody'

        Show
        Pete Wyckoff added a comment - What should fuse return when the group is supergroup then? the current code will end of returning 'nobody'
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I think the correct semantic is to require that the users and groups in HDFS must exist in the local host when using fuse. So if a user or group is not found locally, throw an exception.

        BTW, what Linux does if a remote directory is mounted to a local machine but some accounts used in the remote directory does not exist locally? We probably should do the same.

        Show
        Tsz Wo Nicholas Sze added a comment - I think the correct semantic is to require that the users and groups in HDFS must exist in the local host when using fuse. So if a user or group is not found locally, throw an exception. BTW, what Linux does if a remote directory is mounted to a local machine but some accounts used in the remote directory does not exist locally? We probably should do the same.
        Hide
        Craig Macdonald added a comment -

        what Linux does if a remote directory is mounted to a local machine but some accounts used in the remote directory does not exist locally?

        Linux reports the uid and gid of the files instead of the username and group name. However, this isnt an option in this scenario, as we the usernames not uids are used by Hadoop.

        I prefer that the code returns nobody if the user is not known.

        Show
        Craig Macdonald added a comment - what Linux does if a remote directory is mounted to a local machine but some accounts used in the remote directory does not exist locally? Linux reports the uid and gid of the files instead of the username and group name. However, this isnt an option in this scenario, as we the usernames not uids are used by Hadoop. I prefer that the code returns nobody if the user is not known.
        Hide
        Pete Wyckoff added a comment -

        It looks like we have consensus that this is the right patch.

        +1 on the code. Not really unit test feasible, so i think we can do without that.

        Show
        Pete Wyckoff added a comment - It looks like we have consensus that this is the right patch. +1 on the code. Not really unit test feasible, so i think we can do without that.
        Hide
        Pete Wyckoff added a comment -

        patch for the new organization on trunk. Same code.

        Show
        Pete Wyckoff added a comment - patch for the new organization on trunk. Same code.
        Hide
        Pete Wyckoff added a comment -

        note the hudson test failure was unrelated to this, so i think this is ready to commit.

        Show
        Pete Wyckoff added a comment - note the hudson test failure was unrelated to this, so i think this is ready to commit.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I just committed this. Thanks, Brian!

        Show
        Tsz Wo Nicholas Sze added a comment - I just committed this. Thanks, Brian!
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #680 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/680/ )

          People

          • Assignee:
            Brian Bockelman
            Reporter:
            Brian Bockelman
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development