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. Text File
            HADOOP-9119-2.patch
            8 kB
            Steve Loughran
          2. Text File
            HADOOP-9119.patch
            7 kB
            Steve Loughran

            Issue Links

            incorporates

            Improvement - An improvement or enhancement to an existing feature or task. HADOOP-9118 FileSystemContractBaseTest test data for read/write isn't rigorous enough

            • Trivial - Cosmetic problem like misspelt words or misaligned text.
            • Closed
            is depended upon by

            New Feature - A new feature of the product, which has yet to be developed. HADOOP-8545 Filesystem Implementation for OpenStack Swift

            • Major - Major loss of function.
            • Closed

              Activity

              Ascending order - Click to sort in descending order
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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