Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      UNIX-style sticky bit implemented for HDFS directories. When the sticky bit is set on a directory, files in that directory may be deleted or renamed only by a superuser or the file's owner.

      Description

      Our users (especially Pig) heavily use /tmp for temporary storage.
      Permission are set to 777.

      However, this means any users can rename and also remove (by moving to .Trash) other users directories and files.
      It would be nice if we can have a sticky bit like unix.

      Copy&Pasted from manpage.

      STICKY DIRECTORIES
      When the sticky bit is set on a directory, files in that directory may be unlinked or renamed only by
      root or their owner. Without the sticky bit, anyone able to write to the directory can delete or rename
      files. The sticky bit is commonly found on directories, such as /tmp, that are world-writable.

      1. HADOOP-3953.patch
        55 kB
        Jakob Homan
      2. HADOOP-3953.patch
        62 kB
        Jakob Homan
      3. HADOOP-3953.patch
        42 kB
        Jakob Homan
      4. HADOOP-3953.patch
        39 kB
        Jakob Homan

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          3d 3h 59m 3 Jakob Homan 08/Jan/09 23:00
          Open Open Patch Available Patch Available
          149d 2h 33m 4 Jakob Homan 13/Jan/09 23:40
          Patch Available Patch Available Resolved Resolved
          1d 2h 1 Tsz Wo Nicholas Sze 15/Jan/09 01:41
          Resolved Resolved Closed Closed
          586d 18h 52m 1 Tom White 24/Aug/10 21:34
          Benoy Antony made changes -
          Attachment stickytests [ 12491965 ]
          Benoy Antony made changes -
          Attachment permissiontests [ 12491964 ]
          Benoy Antony made changes -
          Attachment stickytests [ 12491965 ]
          Hide
          Benoy Antony added a comment -

          Stickytests

          Show
          Benoy Antony added a comment - Stickytests
          Benoy Antony made changes -
          Attachment permissiontests [ 12491964 ]
          Hide
          Benoy Antony added a comment -

          FilePermissionTests

          Show
          Benoy Antony added a comment - FilePermissionTests
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Robert Chansler made changes -
          Release Note Implement sticky bit for directories in HDFS. UNIX-style sticky bit implemented for HDFS directories. When the sticky bit is set on a directory, files in that directory may be deleted or renamed only by a superuser or the file's owner.
          Hide
          Robert Chansler added a comment -

          Editorial pass over all release notes prior to publication of 0.21.

          Show
          Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21.
          Owen O'Malley made changes -
          Component/s dfs [ 12310710 ]
          Jakob Homan made changes -
          Release Note Implement sticky bit for directories in HDFS. Is incompatible as prior versions of Hadoop will not be able to read the persisted mode value. Implement sticky bit for directories in HDFS.
          Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
          Hide
          Raghu Angadi added a comment -

          Jakob, release notes sounds much scarier than it should be. Just selecting an incompatible flag is enough, I think.

          Show
          Raghu Angadi added a comment - Jakob, release notes sounds much scarier than it should be. Just selecting an incompatible flag is enough, I think.
          Jakob Homan made changes -
          Release Note Implement sticky bit for directories in HDFS. Is incompatible as prior versions of Hadoop will not be able to read the persisted mode value.
          Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
          Tsz Wo Nicholas Sze made changes -
          Resolution Fixed [ 1 ]
          Fix Version/s 0.21.0 [ 12313563 ]
          Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I just committed this. Thanks, Jakob!

          This needs release note. Please fill it out.

          Show
          Tsz Wo Nicholas Sze added a comment - I just committed this. Thanks, Jakob! This needs release note. Please fill it out.
          Hide
          Jakob Homan added a comment -

          All local unit tests passed except the known-failure documented in HADOOP-4907.

          Show
          Jakob Homan added a comment - All local unit tests passed except the known-failure documented in HADOOP-4907 .
          Tsz Wo Nicholas Sze made changes -
          Hadoop Flags [Incompatible change] [Incompatible change, Reviewed]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Please post the local unit test results when they are ready.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good. Please post the local unit test results when they are ready.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Jakob Homan made changes -
          Attachment HADOOP-3953.patch [ 12397837 ]
          Hide
          Jakob Homan added a comment -
          • Addressed Nicholas' comments
          • Refined chmod processing
          • Tightened up testing
                 [exec] +1 overall.  
                 [exec] 
                 [exec]     +1 @author.  The patch does not contain any @author tags.
                 [exec] 
                 [exec]     +1 tests included.  The patch appears to include 10 new or modified tests.
                 [exec] 
                 [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
                 [exec] 
                 [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
                 [exec] 
                 [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
                 [exec] 
                 [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
            
          Show
          Jakob Homan added a comment - Addressed Nicholas' comments Refined chmod processing Tightened up testing [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 10 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          Jakob Homan made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -
          • The changes in FSDirectory should be reverted since the case is already taken care in INodeFile.
          • For the unit tests, leaving empty catch block is generally a bad practice since there might be some unexpected exceptions. For example, in the codes below, it is correct only if ioe is a sticky bit related exception. It is incorrect if some other IOException like FileNotFoundException is thrown.
            +      try {
            +        hdfs.rename(file, new Path(tmpPath2, "renamed"));
            +        fail("Shouldn't be able to rename someone else's file with SB on");
            +      } catch (IOException ioe) {
            +        // Correct
            +      }
            

            It would be great if you can combine some of them together, so that they take less execution time. I am fine if you don't want to change the tests although I strongly recommend to do so.

          Sorry for not seeing these problems in my previous review.

          Show
          Tsz Wo Nicholas Sze added a comment - The changes in FSDirectory should be reverted since the case is already taken care in INodeFile. For the unit tests, leaving empty catch block is generally a bad practice since there might be some unexpected exceptions. For example, in the codes below, it is correct only if ioe is a sticky bit related exception. It is incorrect if some other IOException like FileNotFoundException is thrown. + try { + hdfs.rename(file, new Path(tmpPath2, "renamed" )); + fail( "Shouldn't be able to rename someone else 's file with SB on" ); + } catch (IOException ioe) { + // Correct + } It would be great if you can combine some of them together, so that they take less execution time. I am fine if you don't want to change the tests although I strongly recommend to do so. Sorry for not seeing these problems in my previous review.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Jakob Homan made changes -
          Attachment HADOOP-3953.patch [ 12397358 ]
          Hide
          Jakob Homan added a comment -

          Updated with Nicholas' suggestions.

               [exec] +1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 8 new or modified tests.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec] 
               [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          
          Show
          Jakob Homan added a comment - Updated with Nicholas' suggestions. [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 8 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          Jakob Homan made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          patch looks good to me. Only one nit: FsPermission.set(FsAction u, FsAction g, FsAction o) is not useful anymore, please remove it.

          Show
          Tsz Wo Nicholas Sze added a comment - patch looks good to me. Only one nit: FsPermission.set(FsAction u, FsAction g, FsAction o) is not useful anymore, please remove it.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Jakob Homan made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Jakob Homan made changes -
          Attachment HADOOP-3953.patch [ 12397137 ]
          Hide
          Jakob Homan added a comment -

          Updated user docs and shell help. Ready for review.

          Show
          Jakob Homan added a comment - Updated user docs and shell help. Ready for review.
          Jakob Homan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Jakob Homan made changes -
          Hadoop Flags [Incompatible change]
          Jakob Homan made changes -
          Attachment HADOOP-3953.patch [ 12397133 ]
          Hide
          Jakob Homan added a comment -

          Here's an initial patch for review. This includes the sticky bit implementation and unit tests. I still need to go through the user documentation and update that; I'll do that now and update the patch shortly with just the documentation changes.

          Show
          Jakob Homan added a comment - Here's an initial patch for review. This includes the sticky bit implementation and unit tests. I still need to go through the user documentation and update that; I'll do that now and update the patch shortly with just the documentation changes.
          Arun C Murthy made changes -
          Link This issue relates to HADOOP-4487 [ HADOOP-4487 ]
          Hide
          Arun C Murthy added a comment -

          To keep Unix-like semantics, it seems reasonable to be able to set the sticky bit on a file (compared to a directory), but that doing so will have no effect on the file. Unless anyone has a good reason to deviate?

          +1 with the caveat that we heavily document the meaning of sticky bit for files.

          Show
          Arun C Murthy added a comment - To keep Unix-like semantics, it seems reasonable to be able to set the sticky bit on a file (compared to a directory), but that doing so will have no effect on the file. Unless anyone has a good reason to deviate? +1 with the caveat that we heavily document the meaning of sticky bit for files.
          Hide
          Jakob Homan added a comment -

          To keep Unix-like semantics, it seems reasonable to be able to set the sticky bit on a file (compared to a directory), but that doing so will have no effect on the file. Unless anyone has a good reason to deviate?

          Show
          Jakob Homan added a comment - To keep Unix-like semantics, it seems reasonable to be able to set the sticky bit on a file (compared to a directory), but that doing so will have no effect on the file. Unless anyone has a good reason to deviate?
          Jakob Homan made changes -
          Field Original Value New Value
          Assignee Jakob Homan [ jghoman ]
          Hide
          Allen Wittenauer added a comment -

          We just had a user wipe out /tmp.

          When are we getting this capability?

          Show
          Allen Wittenauer added a comment - We just had a user wipe out /tmp. When are we getting this capability?
          Hide
          Hemanth Yamijala added a comment -

          +1. When working on HOD, it seemed like this would be very useful in some cases where we wanted shared space for all users.

          Show
          Hemanth Yamijala added a comment - +1. When working on HOD, it seemed like this would be very useful in some cases where we wanted shared space for all users.
          Koji Noguchi created issue -

            People

            • Assignee:
              Jakob Homan
              Reporter:
              Koji Noguchi
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development