Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1968

Enhance TestWriteRead to support File Append and Position Read

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Desirable to enhance TestWriteRead to support command line options to do:
      (1) File Append
      (2) Position Read (currently supporting sequential read).

      1. TestWriteRead.patch
        16 kB
        CW Chung
      2. TestWriteRead.patch
        21 kB
        CW Chung
      3. TestWriteRead.patch
        18 kB
        CW Chung
      4. TestWriteRead.patch
        18 kB
        CW Chung
      5. TestWriteRead-1-Format.patch
        13 kB
        CW Chung
      6. TestWriteRead-2-Append.patch
        16 kB
        CW Chung

        Activity

        Hide
        CW Chung added a comment -

        The patch support the following additional command line options:

        [-append | -truncate]: if file already exists, truncate to 0 length before writing to the file. Default is append to file.

        [-usePosRead | - useSeqRead]: use position read to read the file, or the default sequential read.

        Note: Sorry for the large number of difference. The Eclipse Format did a number of cosmetic changes in the spacing.

        Show
        CW Chung added a comment - The patch support the following additional command line options: [-append | -truncate] : if file already exists, truncate to 0 length before writing to the file. Default is append to file. [-usePosRead | - useSeqRead] : use position read to read the file, or the default sequential read. Note: Sorry for the large number of difference. The Eclipse Format did a number of cosmetic changes in the spacing.
        Hide
        CW Chung added a comment -

        This time with the proper granting license to ASF.

        Show
        CW Chung added a comment - This time with the proper granting license to ASF.
        Hide
        CW Chung added a comment -

        Improvement over the last patch:
        1. Throw exception immediately on error
        2. writeData routine will throw exception if actual data written is not the same amount as request
        3. Print out more debugging info

        Show
        CW Chung added a comment - Improvement over the last patch: 1. Throw exception immediately on error 2. writeData routine will throw exception if actual data written is not the same amount as request 3. Print out more debugging info
        Hide
        John George added a comment -

        This is a very useful test. It does seem to test a lot of corner cases related to flush and "visible byte" reads.
        I do see that this test has a lot of capability, so most of my comments are more related to utilizing that part. As for the code itself, I found the changes useful.

        1. Since this test already has the capability to test position reads, can that also be enabled as a separate @test? Especially since that was something that brought out HDFS-2021 and HDFS-1907.

        2. Since it has the capability to do fc, could we also enable that as another @test?

        3. Should the WR_NTIMES and WR_CHUNK_SIZE be increased to reflect the failed case of HDFS-2021 and HDFS-1907? We can make this JIRA a dependent of those and thus possibly avoid having any test failures.

        Show
        John George added a comment - This is a very useful test. It does seem to test a lot of corner cases related to flush and "visible byte" reads. I do see that this test has a lot of capability, so most of my comments are more related to utilizing that part. As for the code itself, I found the changes useful. 1. Since this test already has the capability to test position reads, can that also be enabled as a separate @test? Especially since that was something that brought out HDFS-2021 and HDFS-1907 . 2. Since it has the capability to do fc, could we also enable that as another @test? 3. Should the WR_NTIMES and WR_CHUNK_SIZE be increased to reflect the failed case of HDFS-2021 and HDFS-1907 ? We can make this JIRA a dependent of those and thus possibly avoid having any test failures.
        Hide
        Konstantin Boudnik added a comment -

        CW, the patch contains lotta white space changes which makes it kinda hard to follow. Could you post an update version which focuses on the meaningful changes only?

        Show
        Konstantin Boudnik added a comment - CW, the patch contains lotta white space changes which makes it kinda hard to follow. Could you post an update version which focuses on the meaningful changes only?
        Hide
        John George added a comment -

        CW, ignore my comment #3, It will be better if those changes are made in the corresponding JIRAs themselves to avoid any dependency between these JIRAS.

        Show
        John George added a comment - CW, ignore my comment #3, It will be better if those changes are made in the corresponding JIRAs themselves to avoid any dependency between these JIRAS.
        Hide
        John George added a comment -

        If you like you can do #2 as another JIRA.
        #1 should actually be part of the corresponding JIRAs that you filed and hence you can ignore that too.

        Show
        John George added a comment - If you like you can do #2 as another JIRA. #1 should actually be part of the corresponding JIRAs that you filed and hence you can ignore that too.
        Hide
        CW Chung added a comment -

        I have patch available to address Comment # 1 by John. To address the comment of Cos, I am dividing the patch into two parts:
        a. A patch (this file) to just re-format of the existing svn copy (basically eclipse format + some manual fix up). Since there is no material code change here, the hope is to get this committed quickly, then step b can be started.
        b. I then generate a patch on a (basically diff my latest version with the new formatted version). This patch would require real review.

        Show
        CW Chung added a comment - I have patch available to address Comment # 1 by John. To address the comment of Cos, I am dividing the patch into two parts: a. A patch (this file) to just re-format of the existing svn copy (basically eclipse format + some manual fix up). Since there is no material code change here, the hope is to get this committed quickly, then step b can be started. b. I then generate a patch on a (basically diff my latest version with the new formatted version). This patch would require real review.
        Hide
        CW Chung added a comment -

        This is part 2 of the patch. This is the material change portion of code from svn. It is a diff from part 1 (which consist of only the formatting change).

        So to reveal / commit, apply the 2 patches in this order:
        a. Apply patch TestWriteRead-1-Format.patch
        to get to the version with better formating

        b. Apply patch TestWriteRead-2-Append.patch to get to the version with material changes.

        (Sorry for the formatting trouble. Next time I'll either do eclipse format right from the start, or never do it! )

        Show
        CW Chung added a comment - This is part 2 of the patch. This is the material change portion of code from svn. It is a diff from part 1 (which consist of only the formatting change). So to reveal / commit, apply the 2 patches in this order: a. Apply patch TestWriteRead-1-Format.patch to get to the version with better formating b. Apply patch TestWriteRead-2-Append.patch to get to the version with material changes. (Sorry for the formatting trouble. Next time I'll either do eclipse format right from the start, or never do it! )
        Hide
        CW Chung added a comment -

        Only one patch is allowed. The formating part was taken care by HDFS-2024.
        This patch contains material changes only.

        Show
        CW Chung added a comment - Only one patch is allowed. The formating part was taken care by HDFS-2024 . This patch contains material changes only.
        Hide
        John George added a comment -

        +1

        Show
        John George added a comment - +1
        Hide
        Hadoop QA added a comment -

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

        +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 (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 failed these core unit tests:

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//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/12481192/TestWriteRead.patch against trunk revision 1130381. +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 (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 failed these core unit tests: +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/687//console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this. Thanks, CW!

        Also, thanks John for the review.

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, CW! Also, thanks John for the review.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #704 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/704/)
        HDFS-1968. Enhance TestWriteRead to support position/sequential read, append, truncate and verbose options. Contributed by CW Chung

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1130667
        Files :

        • /hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestWriteRead.java
        • /hadoop/hdfs/trunk/CHANGES.txt
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #704 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/704/ ) HDFS-1968 . Enhance TestWriteRead to support position/sequential read, append, truncate and verbose options. Contributed by CW Chung szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1130667 Files : /hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestWriteRead.java /hadoop/hdfs/trunk/CHANGES.txt
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #686 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/686/)
        HDFS-1968. Enhance TestWriteRead to support position/sequential read, append, truncate and verbose options. Contributed by CW Chung

        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1130667
        Files :

        • /hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestWriteRead.java
        • /hadoop/hdfs/trunk/CHANGES.txt
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #686 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/686/ ) HDFS-1968 . Enhance TestWriteRead to support position/sequential read, append, truncate and verbose options. Contributed by CW Chung szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1130667 Files : /hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestWriteRead.java /hadoop/hdfs/trunk/CHANGES.txt

          People

          • Assignee:
            CW Chung
            Reporter:
            CW Chung
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development