You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-issues@hadoop.apache.org by "Hairong Kuang (JIRA)" <ji...@apache.org> on 2009/07/28 19:07:14 UTC

[jira] Created: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Redesisn DataNode volumeMap to include all types of Replicas
------------------------------------------------------------

                 Key: HDFS-509
                 URL: https://issues.apache.org/jira/browse/HDFS-509
             Project: Hadoop HDFS
          Issue Type: Sub-task
          Components: data-node
            Reporter: Hairong Kuang
            Assignee: Hairong Kuang
             Fix For: 0.21.0


With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
1. define the in-memory representation of these five types of replicas
2. define the on-disk representation of these replicas
2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12740790#action_12740790 ] 

Hairong Kuang commented on HDFS-509:
------------------------------------

Thanks Konstantin and Suresh for their detailed comments.I've already discussed with Suresh in person regarding his comments and will incorporate most of his suggestions.

> ReplicaBeingWritten has a list of threads in your patch. I do not understand whether you want to let multiple clients write to the same replica. Why do we need that?
 This is the legacy code and I will change this when I work on the pipeline. For this issue I do not want to change any of the write logic code.

I am still debating on the replica class hierarchy. I feel that using the state flay is quite error-prone. Would the following hierarchy provide a good tradeoff, clean but not too complicated. This design removes all interfaces defined in the previous patch.
Block
      ReplicaInfo
                  FinalizedReplica
                  ReplicaWaitingToBeRecovered
                  ReplicaUnderRecovery
                  ReplicaInPipeline
                         ReplicaBeingWritten
                         TemporaryReplica

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment: replicas3.patch

Uploading an updated patch...

> Why is ReplicaState defined in HDFSConstants.java. Is it better to define it in ReplicaInfo?
It's going to be used by NameNode as well.

> ReplicasMap.map should be sized to some reasonable initial size, say, 1024 or 4096. Currently it uses default size of 16.
I will leave it as it is now. It has been like this since the start of the project.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesign DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Summary: Redesign DataNode volumeMap to include all types of Replicas  (was: Redesisn DataNode volumeMap to include all types of Replicas)

> Redesign DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment:     (was: replicas1.patch)

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment: replicas1.patch

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12740791#action_12740791 ] 

Hairong Kuang commented on HDFS-509:
------------------------------------

Repost the class hierarchy:
{noformat}
Block
     ReplicaInfo
          FinalizedReplica
          ReplicaWaitingToBeRecovered
         ReplicaUnderRecovery
         ReplicaInPipeline
               ReplicaBeingWritten
               TemporaryReplica
{noformat}


> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739217#action_12739217 ] 

Hairong Kuang commented on HDFS-509:
------------------------------------

In this patch, the replicas map is a hash map from Long to ReplicaInfo where the key is a block id. If the patch to HADOOP-512 gets committed, I'd like to change the key to be ReplicaInfo so eliminating Long objects to be allocated for keys. It will also eliminate the allocation of a temporary Long object in both get & remove methods.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

      Resolution: Fixed
    Hadoop Flags: [Reviewed]
          Status: Resolved  (was: Patch Available)

I've committed this.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739654#action_12739654 ] 

Suresh Srinivas commented on HDFS-509:
--------------------------------------

Comments:
# Earlier implementation of {{FSDataset.volumeMap}} could find a block in the map in two ways:
#* When block ID and generation stamp matched of block being looked up matched the block in the map.
#* When block ID and wild card generation stamp matched of block being looked up matched only block IDs.
#* Should the ReplicaMap block based lookup match generation stamp as well?
# {{FSDataset.tryUpdateBlock{}}} can access {{replicaInfo}} if it is null. It also does not handle the case when {{ReplicaInfo}} is not found in {{volumeMap}} well. It should throw an IOException as was done earlier.
# {{FSDataset.tryUpdateBlock()}} should not throw exception when oldgs == newblock gs, as the case of block truncation can have same gs?
# {{FSDataset.writeToBlock()}} equivalent functionality of removing block from {{ongoingCreates}} does not exist. Isn't the equivalent, changing the block from type to {{BeingWrittenReplicaInfo}} to different type of {{ReplicaInfo}}?
# {{FSDataset.writeToBlock()}} not sure why we create and add {{TemporaryReplica}} to {{volumeMap}}. TemporaryReplica is used only for replication right?
# {{FSDataset.finalizeBlock()}} is not equivalent to the old code. If the block is already finalized, or is not being written to, we should throw an exception as done before? Should this code also update the GS for the replica? Also indent the if code block right.
# {{FSDataset.unfinalizeBlock()}} should there be a check for ReplicaBeingWritten and return at the beginning. Also the old code removed the block from volumeMap. The code does not remove the replica from the map. Also indentation of this method needs to be fixed.
# {{FSDataset.isValidBlock()}} semantics is completely changed from previous implementation. Is this intentional?
# {{FSDataset.invalidate()}} the check done to compare generation stamp should be done in {{ReplicaInfoMap.get(Block b)}} it self. Just like previous implemenation. Also {{ReplicaInfoMap.remove(long blockId)}} has the same issue of not comparing GS.
# {{BeingWrittenReplicaInfo.java}} - add apache license header
# DataNode.java - recoverBlock used to look up a block in {{ongoingRecovery}} based on blockID only. Now it includes GS. Is this intentional?
# {{PersistentReplica.java()}} where possible constructors taking less params should use a constructor taking more params with default param values.
# {{PersistentReplica.toString()}} is commented
# {{PersistentReplica.detachFile()}} the method comment indicates that the temporary files will be recovered, especially on windows. Is that the case? What happens on other operating systems?
# {{ReplicaInfo.java}} had a reference to block File. Now it is changed to point to the directory where the block exists. Any reason for this change?
# ReplicaUnderRecovery.java should this class just subclass from PersistentReplica?

Nits:
# {{ReplicasMap}} instead of doing {{new Long(long param)}}, you can use java autoboxing by just passing long itself
# Add a comment to describe {{ReplicasMap.volumeMap}} (the Long corresponds to BlockID)
# Can {{ReplicasMap.volumeMap}} be renamed just {{map}} or at least different name from {{volumeMap}}
# Konstantin adds a comment when a method is overridden, what super class or interface it is overriding. It is very useful for reading the code. This is especially useful with complicated hierarchy of ReplicaInfo classes and interfaces.
# ReplicaBeingWritten.java has indenting issue for the first constructor


I will review the tests in the next iteration.


> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment: replicas1.patch

This patch incorporates most of the review comments. Major changes are:
1. ReplicasMap provides two sets of APIs. One set takes Block as an input parameter and needs to match block id as well as block generation stamp. Another set takes block id as input parameter and does not need to match generation stamp.
2. Reorganize Replica class hierarchy as follow:
{noformat}
Block
     ReplicaInfo
          FinalizedReplica
          ReplicaWaitingToBeRecovered
          ReplicaUnderRecovery
          ReplicaInPipeline, i.e. TemporaryReplica
               ReplicaBeingWritten               
{noformat}


> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Status: Patch Available  (was: Open)

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742213#action_12742213 ] 

Hadoop QA commented on HDFS-509:
--------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12416276/replicas3.patch
  against trunk revision 803337.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 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 warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

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

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

Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/62/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/62/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/62/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/62/console

This message is automatically generated.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742004#action_12742004 ] 

Suresh Srinivas commented on HDFS-509:
--------------------------------------

# Why is {{ReplicaState}} defined in HDFSConstants.java. Is it better to define it in ReplicaInfo?
# {{FSDir.getVolumeMap()}} - Log may be confusing if the generation stamps are the same. May need to be reworded. For example "Replacing a block that exists already..."
# {{FSDir.findBlockFile()}} - not sure what the need for debug log is. It does not print any useful information.
# {{FSDataSet.tryUpdateBlock()}} - comments talks about ongoing create threads which was older implementation. Update to reflect new implemenation.
# {{ReplicasMap.map}} should be sized to some reasonable initial size, say, 1024 or 4096. Currently it uses default size of 16.
# {{FSDataSet.tryUpdateBlock()}} - check for replicaInfo == null can be made right after getting it from volumeMap and removing later checks for null. Makes code more readable.


> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742176#action_12742176 ] 

Suresh Srinivas commented on HDFS-509:
--------------------------------------

+1 for the patch

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch, replicas3.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12739864#action_12739864 ] 

Konstantin Shvachko commented on HDFS-509:
------------------------------------------

This is a very complicated hierarchy of classes.
{code}
Block
	ReplicaInfo
		TemporaryReplica
		ReplicaUnderRecovery
		PersistentReplica
			FinalizedReplica
			ReplicaWaitingToBeRecovered
			ReplicaBeingWritten
{code}

I propose to simplify the hierarchy by introducing a ReplicaModificationState{FINALIZED, RBW, RWR, RUR, TEMPORARY} as a field in ReplicaInfo, which identifies the state of the replica while it is going through modifications.
So the hierarchy will look like this
{code}
Block
	ReplicaInfo
		ReplicaUnderRecovery
		ReplicaBeingWritten (?)
{code}

# The new ReplicaInfo essentially implements everything you did for PersistentReplica in your patch. In addition it includes the replicaState field.
# getReadLen() is calculated based on the replicaState, same with isFinalized().
# You do not need to use {{instanceof}} in order to check the type of the replica, just call getState() instead.
# FinalizedReplica and ReplicaWaitingToBeRecovered are different only in getReadLen() implementation, so they are done.
# ReplicaUnderRecovery should keep the state of the replica as it was before the recovery. So you will need some new fields and methods to retrieve old values.
# TemporaryReplica is mainly the same as the ReplicaBeingWritten except for the replicaState.
# The two interfaces PersistentReplicaInfo and BeingWrittenReplicaInfo will probably not be necessary after that.
# ReplicaBeingWritten has a list of threads in your patch. I do not understand whether you want to let multiple clients write to the same replica. Why do we need that?
# I'd propose to rename getReadLen() to getVisibleLength() to comply with the design document.



> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Description: 
With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
1. define the in-memory representation of these five types of replicas
2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

  was:
With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
1. define the in-memory representation of these five types of replicas
2. define the on-disk representation of these replicas
2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.


> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: 0.21.0
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment: replicas2.patch

> Right now it seems that FinalizedReplica can be merged with ReplicaInfo.
No, FinalizedReplica is a detachable replica. I don't want to add a field "detached" to ReplicaInfo.

The updated patch incorporates all review comments.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch, replicas2.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Attachment: replicas.patch

Here is an initial patch for review. This patch
1. defines all different types of replicas: temporary, finalized, replica being written, replica waiting to be recovered, replica under recovery.
2. redefines volumeMap to include all types of replicas. removes ongoingCreates map, renames volumeMap to be replicasMap, and redefines the key to replicasMap to be block's ide;
3. changes FSDataSet to use the new map and replicas APIs. 

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: 0.21.0
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12741651#action_12741651 ] 

Konstantin Shvachko commented on HDFS-509:
------------------------------------------

# According to your comment in jira ReplicaWaitingToBeRecovered should be a subclass of ReplicaInfo rather than ReplicaInPipeline as in the patch.
# ReplicaUnderRecovery.replica to ReplicaUnderRecovery.original
# ReplicaInPipeline.bytesAcked to ReplicaInPipeline.bytesAcknowledged, and respective methods too.
# bumpGenerationStamp() should be an overload of setGenerationStamp(). Or use setNewGenerationStamp().
# Comments like
{code}
/**
 * @see ReplicaInfo#getVisibleLen()
 */
{code}
may be replaced by simply {{/** {@inheritDoc} */}}. Or even no JavaDoc comments, which will have the same effect.
# It would be nice to have @Override with a comment saying which class it overloads. With so much inheritance this is particularly useful. E.g. {{@Override // ReplicaInfo}}
# {{ReplicaInfo.toString()}} would be useful to keep. It helps debugging.
# Add new line at the end of new files.
# I'd avoid introducing constructors that are not used at the moment. Say, ReplicaBeingWritten has 5 constructors which are not used anywhere in the code. We should be adding constructors as they are needed. 
# Also it is better to define constructors using {{this.(...)}} going from most general to more specific. That way the common code is implemented only once.

I cannot evaluate usability of your hierarchy of replica classes until there are use cases for all of them. I think we will have to wait with that. 
Right now it seems that FinalizedReplica can be merged with ReplicaInfo.

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch, replicas1.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HDFS-509) Redesisn DataNode volumeMap to include all types of Replicas

Posted by "Hairong Kuang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HDFS-509?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hairong Kuang updated HDFS-509:
-------------------------------

    Affects Version/s: Append Branch
        Fix Version/s:     (was: 0.21.0)
                       Append Branch

> Redesisn DataNode volumeMap to include all types of Replicas
> ------------------------------------------------------------
>
>                 Key: HDFS-509
>                 URL: https://issues.apache.org/jira/browse/HDFS-509
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: data-node
>    Affects Versions: Append Branch
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: Append Branch
>
>         Attachments: replicas.patch
>
>
> With the new append design, replicas at a DataNode could be in one of the five states: Finalized, ReplicaBeingWritten, ReplicaWaitingToBeRecovered, ReplicaUnderRecovery, and Temporay. This jira is to
> 1. define the in-memory representation of these five types of replicas
> 2. redefine volumeMap to include all five types of replicas and remove ongoingCreates map.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.