Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: datanode, hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      There are quite a few integer (int/byte) constants defined in DataTransferProtocol. It is better to replace them by enums.

      1. h501_20090724b.patch
        47 kB
        Tsz Wo Nicholas Sze
      2. h501_20090724.patch
        46 kB
        Tsz Wo Nicholas Sze

        Activity

        Hide
        Tsz Wo Nicholas Sze added a comment -

        h501_20090724.patch: define constants with enum.

        Show
        Tsz Wo Nicholas Sze added a comment - h501_20090724.patch: define constants with enum.
        Hide
        Tsz Wo Nicholas Sze added a comment -
             [exec] +1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     +1 tests included.  The patch appears to include 9 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        

        Running unit tests ...

        Show
        Tsz Wo Nicholas Sze added a comment - [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 9 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. Running unit tests ...
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Only TestBackupNode failed for "ant test" but it is not related. See HDFS-192.

        Show
        Tsz Wo Nicholas Sze added a comment - Only TestBackupNode failed for "ant test" but it is not related. See HDFS-192 .
        Hide
        Suresh Srinivas added a comment -

        +1

        Nits:

        1. BlockReceiver.java - Indenting of op declaration around line 854, is not indented with the rest of the code (though the existing code does not follow 2 space indentation)
        2. Use read(in) method instead valueOf(in.readByte(). Currently both these methods are used. Perhaps valueOf() could be private. This will also be better in case these fields change from byte to some other type.
        Show
        Suresh Srinivas added a comment - +1 Nits: BlockReceiver.java - Indenting of op declaration around line 854, is not indented with the rest of the code (though the existing code does not follow 2 space indentation) Use read(in) method instead valueOf(in.readByte() . Currently both these methods are used. Perhaps valueOf() could be private. This will also be better in case these fields change from byte to some other type.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        h501_20090724b.patch: incorporated Suresh's comments and cleaned up some codes.

             [exec] +1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     +1 tests included.  The patch appears to include 12 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        Show
        Tsz Wo Nicholas Sze added a comment - h501_20090724b.patch: incorporated Suresh's comments and cleaned up some codes. [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 12 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Suresh Srinivas added a comment -

        +1 for the new patch

        Show
        Suresh Srinivas added a comment - +1 for the new patch
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Got no failure in unit tests this time. (TestBackupNode is really unrelated to this. The patch does not fix it.)

        Show
        Tsz Wo Nicholas Sze added a comment - Got no failure in unit tests this time. (TestBackupNode is really unrelated to this. The patch does not fix it.)
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this.

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #32 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/32/)
        . Use enum to define the constants in DataTransferProtocol.

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #32 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/32/ ) . Use enum to define the constants in DataTransferProtocol.

          People

          • Assignee:
            Tsz Wo Nicholas Sze
            Reporter:
            Tsz Wo Nicholas Sze
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development