You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Konstantin Shvachko (JIRA)" <ji...@apache.org> on 2006/07/28 00:40:13 UTC

[jira] Created: (HADOOP-395) infoPort field should be a DatanodeID member

infoPort field should be a DatanodeID member
--------------------------------------------

                 Key: HADOOP-395
                 URL: http://issues.apache.org/jira/browse/HADOOP-395
             Project: Hadoop
          Issue Type: Improvement
    Affects Versions: 0.4.0
            Reporter: Konstantin Shvachko
         Assigned To: Devaraj Das
             Fix For: 0.5.0


I have a couple of comments on HADOOP-375. Sorry missed that discussion when the patch was under construction.
1) As it is done now infoPort is a member of two classes DatanodeInfo and DatanodeRegistration, which in fact have a
common base DatanodeID. It seems more logical to place infoPort into DatanodeID. This will let us handle the port
assignments inside the constructors rather than outside, and will prevent from unsafe "dynamic casts" like the one
found in FSNamesystem.gotHeartbeat( DatanodeID nodeID, ... )
            nodeinfo.infoPort = ((DatanodeRegistration)nodeID).infoPort;
2) Also, should we make infoPort short rather than integer?
Since it is a part of a heartbeat message we might want keeping it small.
3) Member getters should start with get<MemberName>
- public int infoPort() { return infoPort; }
+ public int getInfoPort() { return infoPort; }


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Resolved: (HADOOP-395) infoPort field should be a DatanodeID member

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/HADOOP-395?page=all ]

Doug Cutting resolved HADOOP-395.
---------------------------------

    Resolution: Fixed

This was fixed as a part of HADOOP-396.

> infoPort field should be a DatanodeID member
> --------------------------------------------
>
>                 Key: HADOOP-395
>                 URL: http://issues.apache.org/jira/browse/HADOOP-395
>             Project: Hadoop
>          Issue Type: Improvement
>    Affects Versions: 0.4.0
>            Reporter: Konstantin Shvachko
>         Assigned To: Devaraj Das
>             Fix For: 0.5.0
>
>         Attachments: refactor_infoport.patch
>
>
> I have a couple of comments on HADOOP-375. Sorry missed that discussion when the patch was under construction.
> 1) As it is done now infoPort is a member of two classes DatanodeInfo and DatanodeRegistration, which in fact have a
> common base DatanodeID. It seems more logical to place infoPort into DatanodeID. This will let us handle the port
> assignments inside the constructors rather than outside, and will prevent from unsafe "dynamic casts" like the one
> found in FSNamesystem.gotHeartbeat( DatanodeID nodeID, ... )
>             nodeinfo.infoPort = ((DatanodeRegistration)nodeID).infoPort;
> 2) Also, should we make infoPort short rather than integer?
> Since it is a part of a heartbeat message we might want keeping it small.
> 3) Member getters should start with get<MemberName>
> - public int infoPort() { return infoPort; }
> + public int getInfoPort() { return infoPort; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (HADOOP-395) infoPort field should be a DatanodeID member

Posted by "Devaraj Das (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/HADOOP-395?page=comments#action_12424076 ] 
            
Devaraj Das commented on HADOOP-395:
------------------------------------

Noticed now that Konstantin's patch for Hadoop-396 will make this patch go stale. So I will submit another patch after Konstantin's patch is committed. Please don't commit this patch for now.

> infoPort field should be a DatanodeID member
> --------------------------------------------
>
>                 Key: HADOOP-395
>                 URL: http://issues.apache.org/jira/browse/HADOOP-395
>             Project: Hadoop
>          Issue Type: Improvement
>    Affects Versions: 0.4.0
>            Reporter: Konstantin Shvachko
>         Assigned To: Devaraj Das
>             Fix For: 0.5.0
>
>         Attachments: refactor_infoport.patch
>
>
> I have a couple of comments on HADOOP-375. Sorry missed that discussion when the patch was under construction.
> 1) As it is done now infoPort is a member of two classes DatanodeInfo and DatanodeRegistration, which in fact have a
> common base DatanodeID. It seems more logical to place infoPort into DatanodeID. This will let us handle the port
> assignments inside the constructors rather than outside, and will prevent from unsafe "dynamic casts" like the one
> found in FSNamesystem.gotHeartbeat( DatanodeID nodeID, ... )
>             nodeinfo.infoPort = ((DatanodeRegistration)nodeID).infoPort;
> 2) Also, should we make infoPort short rather than integer?
> Since it is a part of a heartbeat message we might want keeping it small.
> 3) Member getters should start with get<MemberName>
> - public int infoPort() { return infoPort; }
> + public int getInfoPort() { return infoPort; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Updated: (HADOOP-395) infoPort field should be a DatanodeID member

Posted by "Devaraj Das (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/HADOOP-395?page=all ]

Devaraj Das updated HADOOP-395:
-------------------------------

    Attachment: refactor_infoport.patch

Attached is the patch.

> infoPort field should be a DatanodeID member
> --------------------------------------------
>
>                 Key: HADOOP-395
>                 URL: http://issues.apache.org/jira/browse/HADOOP-395
>             Project: Hadoop
>          Issue Type: Improvement
>    Affects Versions: 0.4.0
>            Reporter: Konstantin Shvachko
>         Assigned To: Devaraj Das
>             Fix For: 0.5.0
>
>         Attachments: refactor_infoport.patch
>
>
> I have a couple of comments on HADOOP-375. Sorry missed that discussion when the patch was under construction.
> 1) As it is done now infoPort is a member of two classes DatanodeInfo and DatanodeRegistration, which in fact have a
> common base DatanodeID. It seems more logical to place infoPort into DatanodeID. This will let us handle the port
> assignments inside the constructors rather than outside, and will prevent from unsafe "dynamic casts" like the one
> found in FSNamesystem.gotHeartbeat( DatanodeID nodeID, ... )
>             nodeinfo.infoPort = ((DatanodeRegistration)nodeID).infoPort;
> 2) Also, should we make infoPort short rather than integer?
> Since it is a part of a heartbeat message we might want keeping it small.
> 3) Member getters should start with get<MemberName>
> - public int infoPort() { return infoPort; }
> + public int getInfoPort() { return infoPort; }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira