Hadoop Common
  1. Hadoop Common
  2. HADOOP-9119

Add test to FileSystemContractBaseTest to verify integrity of overwritten files

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.3-alpha
    • Component/s: fs, test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Patches adds more tests to verify overwritten and more complex operations -write-delete-overwrite. By using differently sized datasets and different data inside, these tests verify that the overwrite really did take place. While HDFS meets all these requirements directly, eventually consistent object stores may not -hence these tests.
      Show
      Patches adds more tests to verify overwritten and more complex operations -write-delete-overwrite. By using differently sized datasets and different data inside, these tests verify that the overwrite really did take place. While HDFS meets all these requirements directly, eventually consistent object stores may not -hence these tests.

      Description

      The test FileSystemContractBaseTest.testOverwrite() is meant to verify that overwrites work -but it only overwrites a zero byte file with some data and only checks for length. This can hide problems where there overwrite was somehow corrupted.

      1. HADOOP-9119-2.patch
        8 kB
        Steve Loughran
      2. HADOOP-9119.patch
        7 kB
        Steve Loughran

        Issue Links

          Activity

          Steve Loughran created issue -
          Steve Loughran made changes -
          Field Original Value New Value
          Description The test {FileSystemContractBaseTest.testOverwrite()}} is meant to verify that overwrites work -but it only overwrites a zero byte file with some data and only checks for length. This can hide problems where there overwrite was somehow corrupted. The test {{FileSystemContractBaseTest.testOverwrite()}} is meant to verify that overwrites work -but it only overwrites a zero byte file with some data and only checks for length. This can hide problems where there overwrite was somehow corrupted.
          Hide
          Steve Loughran added a comment -

          I propose new tests that write to the same file with: less data, more data, the same amount of data -and a different set of data in each. The byte-by-byte verification will pick up any situation where the old data is somehow mixed with the new data.

          Show
          Steve Loughran added a comment - I propose new tests that write to the same file with: less data, more data, the same amount of data -and a different set of data in each. The byte-by-byte verification will pick up any situation where the old data is somehow mixed with the new data.
          Steve Loughran made changes -
          Assignee Steve Loughran [ stevel@apache.org ]
          Hide
          Steve Loughran added a comment -

          git am formatted patch

          Show
          Steve Loughran added a comment - git am formatted patch
          Steve Loughran made changes -
          Attachment HADOOP-9119.patch [ 12560374 ]
          Steve Loughran made changes -
          Link This issue incorporates HADOOP-9118 [ HADOOP-9118 ]
          Hide
          Steve Loughran added a comment -

          the dataset() method used to create test datasets takes a broader modulus for its sets.

          Show
          Steve Loughran added a comment - the dataset() method used to create test datasets takes a broader modulus for its sets.
          Steve Loughran made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Release Note Patches adds more tests to verify overwritten and more complex operations -write-delete-overwrite. By using differently sized datasets and different data inside, these tests verify that the overwrite really did take place. While HDFS meets all these requirements directly, eventually consistent object stores may not -hence these 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/12560374/HADOOP-9119.patch
          against trunk revision .

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

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

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1847//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1847//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/12560374/HADOOP-9119.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1847//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1847//console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          Don't think the char to String conversion method is necessary, String#format handles bytes right? Otherwise looks good.

          Show
          Eli Collins added a comment - Don't think the char to String conversion method is necessary, String#format handles bytes right? Otherwise looks good.
          Hide
          Steve Loughran added a comment -

          the toChar() method takes anything >=32 and converts it to its equivalent 33 => ! ,etc, but for anything <32, maps to the hex value, so you can see the difference between expected and actual even for the low values

          Show
          Steve Loughran added a comment - the toChar() method takes anything >=32 and converts it to its equivalent 33 => ! ,etc, but for anything <32, maps to the hex value, so you can see the difference between expected and actual even for the low values
          Steve Loughran made changes -
          Link This issue blocks HADOOP-8545 [ HADOOP-8545 ]
          Hide
          Suresh Srinivas added a comment -

          Comments:

          1. Please add additional comments to toChar() method explaining breifly what is being done.
          2. #writeAndRead() - I may not have understood the code correctly. Some comments:
            • what is the reason why you need to accumulate the error strings in result and why not just print LOG.warn() as error is encountered?
          3. Variable name first_error_line - it is not related to really line right? You could just maintain the index where you encountered error right?
            • Also not sure why have len * 40 as the size of result ro why for loop is between first_error_line - 10 to first_error_line + 10. A brief description of why this is done would help understand the code better.
            • You could have array index of out of exception in the for loop. You should swap Math.min() and Math.max()
          4. Some nits - Some lines have are more than 80 chars
          Show
          Suresh Srinivas added a comment - Comments: Please add additional comments to toChar() method explaining breifly what is being done. #writeAndRead() - I may not have understood the code correctly. Some comments: what is the reason why you need to accumulate the error strings in result and why not just print LOG.warn() as error is encountered? Variable name first_error_line - it is not related to really line right? You could just maintain the index where you encountered error right? Also not sure why have len * 40 as the size of result ro why for loop is between first_error_line - 10 to first_error_line + 10 . A brief description of why this is done would help understand the code better. You could have array index of out of exception in the for loop. You should swap Math.min() and Math.max() Some nits - Some lines have are more than 80 chars
          Hide
          Steve Loughran added a comment -

          I'll fix these. Does the 80-line limit still apply?

          Show
          Steve Loughran added a comment - I'll fix these. Does the 80-line limit still apply?
          Hide
          Suresh Srinivas added a comment -

          Does the 80-line limit still apply?

          Yes.

          Show
          Suresh Srinivas added a comment - Does the 80-line limit still apply? Yes.
          Steve Loughran made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Steve Loughran added a comment -

          Patch addressing Suresh's issues

          Show
          Steve Loughran added a comment - Patch addressing Suresh's issues
          Steve Loughran made changes -
          Attachment HADOOP-9119-2.patch [ 12563740 ]
          Steve Loughran made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563740/HADOOP-9119-2.patch
          against trunk revision .

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

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

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2011//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2011//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/12563740/HADOOP-9119-2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2011//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2011//console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          +1 for the change.

          Show
          Suresh Srinivas added a comment - +1 for the change.
          Suresh Srinivas made changes -
          Issue Type Bug [ 1 ] Test [ 6 ]
          Component/s fs [ 12310689 ]
          Component/s test [ 12311440 ]
          Hide
          Suresh Srinivas added a comment -

          Committed the patch to branch-2 and trunk. Thank you Steve.

          Show
          Suresh Srinivas added a comment - Committed the patch to branch-2 and trunk. Thank you Steve.
          Suresh Srinivas made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s 2.0.3-alpha [ 12323273 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3192 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3192/)
          HADOOP-9119. Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387)

          Result = SUCCESS
          suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3192 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3192/ ) HADOOP-9119 . Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387) Result = SUCCESS suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #91 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/91/)
          HADOOP-9119. Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387)

          Result = SUCCESS
          suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #91 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/91/ ) HADOOP-9119 . Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387) Result = SUCCESS suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1280 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1280/)
          HADOOP-9119. Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387)

          Result = FAILURE
          suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1280 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1280/ ) HADOOP-9119 . Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387) Result = FAILURE suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1308 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1308/)
          HADOOP-9119. Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387)

          Result = FAILURE
          suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1308 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1308/ ) HADOOP-9119 . Add test to FileSystemContractBaseTest to verify integrity of overwritten files. Contributed by Steve Loughran. (Revision 1430387) Result = FAILURE suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1430387 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Link This issue blocks HADOOP-8545 [ HADOOP-8545 ]
          Gavin made changes -
          Link This issue is depended upon by HADOOP-8545 [ HADOOP-8545 ]

            People

            • Assignee:
              Steve Loughran
              Reporter:
              Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development