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

        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.
        Hide
        Pete Wyckoff added a comment -

        old patch doesn't work.

        Show
        Pete Wyckoff added a comment - old patch doesn't work.
        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"} ;
        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.
        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!
        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/ )

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development