Hadoop Common
  1. Hadoop Common
  2. HADOOP-5332

Make support for file append API configurable

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.19.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Appending to HDFS files is supported only if dfs.support.append is set to true.

      Description

      FSNamesystem already has a private dfs.support.append config option. Make this public in src/hdfs/hdfs-default.xml and set the default to false.

      1. disableAppendTrunk.patch
        3 kB
        dhruba borthakur
      2. disableAppendTrunk.patch
        3 kB
        dhruba borthakur
      3. disableAppendTrunk.patch
        5 kB
        dhruba borthakur
      4. disableAppend_0.19.patch
        10 kB
        dhruba borthakur
      5. disableAppendTrunk.patch
        8 kB
        dhruba borthakur
      6. disableAppend_0.19.patch
        12 kB
        dhruba borthakur
      7. disableAppendTrunk.patch
        8 kB
        dhruba borthakur
      8. disableAppend_0.19.patch
        12 kB
        dhruba borthakur
      9. disableAppendTrunk.patch
        7 kB
        dhruba borthakur
      10. disableAppend_0.19.patch
        16 kB
        dhruba borthakur
      11. disableAppendTrunk.patch
        10 kB
        dhruba borthakur

        Issue Links

          Activity

          Hide
          dhruba borthakur added a comment -

          "append" is not supported by default. If you set the config property dfs.support.append, then append is enabled.

          Show
          dhruba borthakur added a comment - "append" is not supported by default. If you set the config property dfs.support.append, then append is enabled.
          Hide
          Nigel Daley added a comment -

          Dhruba, what about the libhdfs append tests?

          Show
          Nigel Daley added a comment - Dhruba, what about the libhdfs append tests?
          Hide
          dhruba borthakur added a comment -

          Make libhdfs unit tests test the append api.

          Show
          dhruba borthakur added a comment - Make libhdfs unit tests test the append api.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 9 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

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

          -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-vesta/7/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/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/12401004/disableAppendTrunk.patch against trunk revision 748090. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 release audit. The applied patch does not increase the total number of release audit warnings. -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-vesta/7/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta/7/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          Fixed unit test failures.

          Show
          dhruba borthakur added a comment - Fixed unit test failures.
          Hide
          Raghu Angadi added a comment -

          Should datanode clean up tmp directory if this config is false (HADOOP-5225)? Seems required to match decisions made in 0.19.1 and 0.18. Otherwise users are not completely isolated.

          Also should the default used in FSNamesystem.java for dfs.support.append to false?

          Show
          Raghu Angadi added a comment - Should datanode clean up tmp directory if this config is false ( HADOOP-5225 )? Seems required to match decisions made in 0.19.1 and 0.18. Otherwise users are not completely isolated. Also should the default used in FSNamesystem.java for dfs.support.append to false?
          Hide
          dhruba borthakur added a comment -

          Hi Raghu, thanks for the info. HADOOP-5225 has gone into 0.19 branch. The patch I attached ot this JIRA is for 0.20 and trunk. If there is consensus that this patch needs to go into 019, then I will create a separate patch for 0.19 branch.

          Show
          dhruba borthakur added a comment - Hi Raghu, thanks for the info. HADOOP-5225 has gone into 0.19 branch. The patch I attached ot this JIRA is for 0.20 and trunk. If there is consensus that this patch needs to go into 019, then I will create a separate patch for 0.19 branch.
          Hide
          dhruba borthakur added a comment -

          Re-triggering hudson

          Show
          dhruba borthakur added a comment - Re-triggering hudson
          Hide
          Raghu Angadi added a comment -

          yes, there needs to be a 0.19 version of this patch (since it is marked for 0.19.2). But I was not talking about it.

          > Hi Raghu, thanks for the info. HADOOP-5225 has gone into 0.19 branch. The patch I attached ot this JIRA is for 0.20 and trunk

          yes. should HADOOP-5225 also go into 0.20 and trunk? I think so, at least until HADOOP-4663 is committed. Two options for this : either marking HADOOP-4663 a blocker for 0.19.2 and 0.20 or get HADOOP-5225 in to this patch.

          Show
          Raghu Angadi added a comment - yes, there needs to be a 0.19 version of this patch (since it is marked for 0.19.2). But I was not talking about it. > Hi Raghu, thanks for the info. HADOOP-5225 has gone into 0.19 branch. The patch I attached ot this JIRA is for 0.20 and trunk yes. should HADOOP-5225 also go into 0.20 and trunk? I think so, at least until HADOOP-4663 is committed. Two options for this : either marking HADOOP-4663 a blocker for 0.19.2 and 0.20 or get HADOOP-5225 in to this patch.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 18 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

          +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 failed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/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/12401073/disableAppendTrunk.patch against trunk revision 748783. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +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 failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/18/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          Patch for 0.19 branch.

          Show
          dhruba borthakur added a comment - Patch for 0.19 branch.
          Hide
          dhruba borthakur added a comment - - edited

          We should commit HADOOP-5225 as it is into 0.20 and trunk as well.

          Show
          dhruba borthakur added a comment - - edited We should commit HADOOP-5225 as it is into 0.20 and trunk as well.
          Hide
          Jim Kellerman added a comment -

          HADOOP-5525 ? I can't find it.

          Show
          Jim Kellerman added a comment - HADOOP-5525 ? I can't find it.
          Hide
          Raghu Angadi added a comment -

          regd 0.19 patch for libhdfs : It should leave the curly braces in that were always there. Otherwise, indentation is not correct.

          Since this jira is about using the config variable for append, I think it is simpler to incorporate HADOOP-5225 here that removes the tmp files only when appends are disabled. A whole new jira and associated overhead could be avoided.

          Show
          Raghu Angadi added a comment - regd 0.19 patch for libhdfs : It should leave the curly braces in that were always there. Otherwise, indentation is not correct. Since this jira is about using the config variable for append, I think it is simpler to incorporate HADOOP-5225 here that removes the tmp files only when appends are disabled. A whole new jira and associated overhead could be avoided.
          Hide
          Nigel Daley added a comment -

          Also, the javadoc for the append method should mention it's new behavior based on the setting of the config variable.

          Show
          Nigel Daley added a comment - Also, the javadoc for the append method should mention it's new behavior based on the setting of the config variable.
          Hide
          dhruba borthakur added a comment -

          Canceling patch to incorporate review comments.

          Show
          dhruba borthakur added a comment - Canceling patch to incorporate review comments.
          Hide
          dhruba borthakur added a comment -

          1. change javadocs to reflect the change in behaviour of ClientProtocol.append
          2. Indent braces in hdfs_test.c
          3. Pulled in fix for HADOOP-5225

          Show
          dhruba borthakur added a comment - 1. change javadocs to reflect the change in behaviour of ClientProtocol.append 2. Indent braces in hdfs_test.c 3. Pulled in fix for HADOOP-5225
          Hide
          Robert Chansler added a comment -

          At the bottom of the patch, the value is "true" but the description says "set to false".

          Is it correct that the default value should be false?

          Show
          Robert Chansler added a comment - At the bottom of the patch, the value is "true" but the description says "set to false". Is it correct that the default value should be false?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12401374/disableAppend_0.19.patch
          against trunk revision 750237.

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

          +1 tests included. The patch appears to include 15 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-minerva.apache.org/6/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/12401374/disableAppend_0.19.patch against trunk revision 750237. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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-minerva.apache.org/6/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          Hudson applied the patch for 0.19 to trunk, hence tests failed.

          Show
          dhruba borthakur added a comment - Hudson applied the patch for 0.19 to trunk, hence tests failed.
          Hide
          dhruba borthakur added a comment -

          ASttachign the same patch as before.

          Show
          dhruba borthakur added a comment - ASttachign the same patch as before.
          Hide
          Eric Yang added a comment -

          Resubmit patch to hudson, trunk test was broken by HADOOP-5409.

          Show
          Eric Yang added a comment - Resubmit patch to hudson, trunk test was broken by HADOOP-5409 .
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Dhruba, as Rob mentioned before, the default value in src/c++/libhdfs/tests/conf/hdfs-site.xml is set to be true but the description saying that it should be false. Which one is correct?

          +  <name>dfs.support.append</name>
          +  <value>true</value>
          +  <description>Does HDFS allow appends to files?
          +               This is currently set to false because there are bugs in the
          +               "append code" and is not supported in any prodction cluster.
          +  </description>
          
          Show
          Tsz Wo Nicholas Sze added a comment - Dhruba, as Rob mentioned before , the default value in src/c++/libhdfs/tests/conf/hdfs-site.xml is set to be true but the description saying that it should be false. Which one is correct? + <name>dfs.support.append</name> + <value>true</value> + <description>Does HDFS allow appends to files? + This is currently set to false because there are bugs in the + "append code" and is not supported in any prodction cluster. + </description>
          Hide
          dhruba borthakur added a comment -

          Changed a comment to to say that the libhdfs unit test is allowing hdfs appends.

          Show
          dhruba borthakur added a comment - Changed a comment to to say that the libhdfs unit test is allowing hdfs appends.
          Hide
          dhruba borthakur added a comment -

          Changed a comment to to say that the libhdfs unit test is allowing hdfs appends.

          Show
          dhruba borthakur added a comment - Changed a comment to to say that the libhdfs unit test is allowing hdfs appends.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 18 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

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

          -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-minerva.apache.org/33/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/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/12401563/disableAppendTrunk.patch against trunk revision 751463. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 release audit. The applied patch does not increase the total number of release audit warnings. -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-minerva.apache.org/33/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-minerva.apache.org/33/console This message is automatically generated.
          Hide
          Nigel Daley added a comment -

          Nicholas, Dhruba, Raghu, is this ready to commit?

          Show
          Nigel Daley added a comment - Nicholas, Dhruba, Raghu, is this ready to commit?
          Hide
          dhruba borthakur added a comment -

          The unit tests failed but most of the failures are unrelated. If Raghu agrees, then I can commit it.

          Show
          dhruba borthakur added a comment - The unit tests failed but most of the failures are unrelated. If Raghu agrees, then I can commit it.
          Hide
          dhruba borthakur added a comment -

          Let me also run the unit tests once again my own workspace

          Show
          dhruba borthakur added a comment - Let me also run the unit tests once again my own workspace
          Hide
          Raghu Angadi added a comment -

          Since append is disabled by default, the tests that needed fixes in HADOOP-5225 should have append enabled. The current patch only has the FSDataset fix. This could explain two of the tests failed.

          Apart from that, +1 for the patch.

          TestPread failures in Hudson seem unrelated.

          Show
          Raghu Angadi added a comment - Since append is disabled by default, the tests that needed fixes in HADOOP-5225 should have append enabled. The current patch only has the FSDataset fix. This could explain two of the tests failed. Apart from that, +1 for the patch. TestPread failures in Hudson seem unrelated.
          Hide
          dhruba borthakur added a comment -

          Incorporate Raghu's review comments.

          Show
          dhruba borthakur added a comment - Incorporate Raghu's review comments.
          Hide
          dhruba borthakur added a comment -

          Fixed the two failed unit tests.

          Show
          dhruba borthakur added a comment - Fixed the two failed unit tests.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12401791/disableAppendTrunk.patch
          against trunk revision 752204.

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

          +1 tests included. The patch appears to include 24 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 Eclipse classpath. The patch retains Eclipse classpath integrity.

          +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-vesta.apache.org/66/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/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/12401791/disableAppendTrunk.patch against trunk revision 752204. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 24 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 Eclipse classpath. The patch retains Eclipse classpath integrity. +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-vesta.apache.org/66/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/66/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          I am going to commit this to 0.19, 0.20 and trunk very shortly.

          Show
          dhruba borthakur added a comment - I am going to commit this to 0.19, 0.20 and trunk very shortly.
          Hide
          dhruba borthakur added a comment -

          I just committed this.

          Show
          dhruba borthakur added a comment - I just committed this.
          Hide
          Hudson added a comment -
          Show
          Hudson added a comment - Integrated in Hadoop-trunk #778 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/778/ )

            People

            • Assignee:
              dhruba borthakur
              Reporter:
              Nigel Daley
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development