hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Owen O'Malley (JIRA)" <j...@apache.org>
Subject [jira] Updated: (HADOOP-321) DatanodeInfo refactoring
Date Wed, 19 Jul 2006 07:25:16 GMT
     [ http://issues.apache.org/jira/browse/HADOOP-321?page=all ]

Owen O'Malley updated HADOOP-321:
---------------------------------

    Attachment: DatanodeInfo_refactor5.patch

I've trivially updated Konstantin's patch to make it apply to the current sources.

I'm still not wild about having the DataNodeInfo send part of itself via RPC or being Comparable.
 There is a lot of confusion still about what should be a DatanodeInfo versus a DatanodeID,
but that was present in the original code, not the patch and indicates additional refactorings
that are needed.

I ran both the unit tests and a distributed test and it was all good.

This patch gets rid of some of the redundant code and that is a good thing. I also like the
comments saying what the types of the elements in the containers are. I'm looking forward
to when we can declare all of the container types. *smile*

A few comments:
  1. A bunch of the routines have empty javadoc, which isn't useful at all. Although I suspect
 those routines were just moved from somewhere else, it makes the patch look bad.
  2. Namenode.sendHeartbeat creates a throwaway DatanodeDescriptor, including its block set,
just to do a lookup on the name. Is there a way to index that table on the DatanodeID instead?
  3. DatanodeID implements compareTo, but not equals or hashCode. Usually it is a good idea
to define all of them.
  4. Why isn't DatanodeID implementing Writable?
  5. To write out strings, you don't need to create UTF8 from them, you just need to do:
      UTF8.writeString(out, name);
      UTF8.writeString(out, storageId);
   and reading looks like:
     name = UTF8.readString(in);
     storageId = UTF8.readString(in);
  It works either way, it is just a little easier to read the code with the helper functions.


> DatanodeInfo refactoring
> ------------------------
>
>                 Key: HADOOP-321
>                 URL: http://issues.apache.org/jira/browse/HADOOP-321
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: dfs
>            Reporter: Konstantin Shvachko
>         Assigned To: Konstantin Shvachko
>             Fix For: 0.5.0
>
>         Attachments: DatanodeInfo_refactor.patch, DatanodeInfo_refactor3.patch, DatanodeInfo_refactor4-comp.patch,
DatanodeInfo_refactor4.patch, DatanodeInfo_refactor5.patch
>
>
> I'm trying to refactor some name node classes, which seem to be similar.
> So DatanodeInfo is a public api now for purely external ( to name node) use.
> The name node class that stores information about data nodes including the
> set of its blocks is called DatanodeDescriptor.
> The DatanodeReport is removed since it was a variation of DatanodeInfo.
> Previously DatanodeInfo and DatanodeDescriptor were the same class, and
> DatanodeReport was used for reporting node statistics only.
> This is a preparation step for HADOOP-306.

-- 
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

        

Mime
View raw message