Hadoop Common
  1. Hadoop Common
  2. HADOOP-3796

fuse-dfs should take rw,ro,trashon,trashoff,protected=blah mount arguments rather than them being compiled in

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.19.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Changed Fuse configuration to use mount options.

      Description

      Right now read only versus read write and other options are compile time. We should change these to be options just like any other mount options.

      1. patch0.txt
        19 kB
        Pete Wyckoff
      2. patch1.txt
        16 kB
        Pete Wyckoff
      3. patch2.txt
        23 kB
        Pete Wyckoff

        Activity

        Pete Wyckoff created issue -
        Pete Wyckoff made changes -
        Field Original Value New Value
        Attachment patch0.txt [ 12387062 ]
        Pete Wyckoff made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        dhruba borthakur added a comment -

        Does it make sense to make the default setting to be "readwrite"? i.e. if no mount options are specified, then it is mounted 'readwrite'.

        Show
        dhruba borthakur added a comment - Does it make sense to make the default setting to be "readwrite"? i.e. if no mount options are specified, then it is mounted 'readwrite'.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12387062/patch0.txt
        against trunk revision 680577.

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

        +1 tests included. The patch appears to include 5 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2964/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/12387062/patch0.txt against trunk revision 680577. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 5 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2964/console This message is automatically generated.
        Hide
        Pete Wyckoff added a comment -

        fix patch's problem with fuse wrapper.

        Show
        Pete Wyckoff added a comment - fix patch's problem with fuse wrapper.
        Pete Wyckoff made changes -
        Attachment patch1.txt [ 12387068 ]
        Hide
        Pete Wyckoff added a comment -

        old patch doesn't work.

        Show
        Pete Wyckoff added a comment - old patch doesn't work.
        Pete Wyckoff made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Pete Wyckoff made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Pete Wyckoff added a comment -

        Yes, the default is read_only. The way the fuse opts macros work is not to supply a default, but rather a value if the flag is set. And, the options struct is bzero-d in main. Maybe could be more explicit about setting values to 0, but then if I don't bzero it, we could miss initializing something else later??

        Show
        Pete Wyckoff added a comment - Yes, the default is read_only. The way the fuse opts macros work is not to supply a default, but rather a value if the flag is set. And, the options struct is bzero-d in main. Maybe could be more explicit about setting values to 0, but then if I don't bzero it, we could miss initializing something else later??
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12387068/patch1.txt
        against trunk revision 680577.

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +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 release audit. The applied patch does not increase the total number of release audit warnings.

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/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/12387068/patch1.txt against trunk revision 680577. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2966/console This message is automatically generated.
        Hide
        dhruba borthakur added a comment -

        Ok, my comment was actually a question to ask what is a good default? Nfs default mount is read-write. Is there a reason why the default mount for fuse-dfs is read-only?

        Show
        dhruba borthakur added a comment - Ok, my comment was actually a question to ask what is a good default? Nfs default mount is read-write. Is there a reason why the default mount for fuse-dfs is read-only?
        Hide
        Pete Wyckoff added a comment -

        Sorry, I meant to say the default is read_only = 0, so rw.

        Show
        Pete Wyckoff added a comment - Sorry, I meant to say the default is read_only = 0, so rw.
        Hide
        Zheng Shao added a comment -

        nitpick:

        Index: src/contrib/fuse-dfs/src/test/TestFuseDFS.java

        • String cmd[] = new String[4];
          + String cmd[] = new String[6];
          ...
          + cmd[index++] = "debug";
          + cmd[index++] = "entry_timeout=1";
          + cmd[index++] = "attribute_timeout=1";

        It's better to write like this to avoid the potential mismatch of 6 and the number of index++ in the future:

        String cmd[] = new String[]

        {"a", "b", "c"}

        ;

        Show
        Zheng Shao added a comment - nitpick: Index: src/contrib/fuse-dfs/src/test/TestFuseDFS.java String cmd[] = new String [4] ; + String cmd[] = new String [6] ; ... + cmd [index++] = "debug"; + cmd [index++] = "entry_timeout=1"; + cmd [index++] = "attribute_timeout=1"; It's better to write like this to avoid the potential mismatch of 6 and the number of index++ in the future: String cmd[] = new String[] {"a", "b", "c"} ;
        Owen O'Malley made changes -
        Assignee Pete Wyckoff [ wyckoff ]
        Hide
        Pete Wyckoff added a comment -

        cleaned up tests and fixed some bugs and also changed the README.

        Show
        Pete Wyckoff added a comment - cleaned up tests and fixed some bugs and also changed the README.
        Pete Wyckoff made changes -
        Attachment patch2.txt [ 12387252 ]
        Pete Wyckoff made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Pete Wyckoff made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Pete Wyckoff made changes -
        Release Note This changes how fuse dfs is configured. Moved from doing things with defines at compile time to using mount options - i.e., arguments. Compile time options now need to be put in fstab - see README.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12387252/patch2.txt
        against trunk revision 681243.

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

        +1 tests included. The patch appears to include 8 new or modified tests.

        +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 release audit. The applied patch does not increase the total number of release audit warnings.

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/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/12387252/patch2.txt against trunk revision 681243. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2992/console This message is automatically generated.
        Hide
        Pete Wyckoff added a comment -

        Zheng can you have another look and comment or +1 it?

        This isn't the greatest solution since most flags will need -o preceding them e.g., -ousetrash, but that's the way it is in the fuse world. Normally this is fine, but in fstab, it is a bit weird to have -ousetrash,-oprivate,-o... The alternative is we don't use fuse's option handling capabilities and use our own but i think that's even worse.

        Show
        Pete Wyckoff added a comment - Zheng can you have another look and comment or +1 it? This isn't the greatest solution since most flags will need -o preceding them e.g., -ousetrash, but that's the way it is in the fuse world. Normally this is fine, but in fstab, it is a bit weird to have -ousetrash,-oprivate,-o... The alternative is we don't use fuse's option handling capabilities and use our own but i think that's even worse.
        Hide
        Zheng Shao added a comment -

        +1
        Looks good to me.

        Show
        Zheng Shao added a comment - +1 Looks good to me.
        Hide
        dhruba borthakur added a comment -

        I just committed this. Thanks Pete!

        Show
        dhruba borthakur added a comment - I just committed this. Thanks Pete!
        dhruba borthakur made changes -
        Hadoop Flags [Incompatible change] [Incompatible change, Reviewed]
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Fix Version/s 0.19.0 [ 12313211 ]
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #581 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/581/ )
        Robert Chansler made changes -
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Release Note This changes how fuse dfs is configured. Moved from doing things with defines at compile time to using mount options - i.e., arguments. Compile time options now need to be put in fstab - see README. Changed Fuse configuration to use mount options.
        Nigel Daley made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Owen O'Malley made changes -
        Component/s contrib/fuse-dfs [ 12312376 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        2d 3h 49m 2 Pete Wyckoff 31/Jul/08 02:40
        Open Open Patch Available Patch Available
        9d 22h 11m 3 Pete Wyckoff 31/Jul/08 02:40
        Patch Available Patch Available Resolved Resolved
        1d 4h 54m 1 dhruba borthakur 01/Aug/08 07:34
        Resolved Resolved Closed Closed
        111d 16h 3m 1 Nigel Daley 20/Nov/08 23:38

          People

          • Assignee:
            Pete Wyckoff
            Reporter:
            Pete Wyckoff
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development