[ 
https://issues.apache.org/jira/browse/HDFS-7369?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14361131#comment-14361131
 ] 

Kai Zheng commented on HDFS-7369:
---------------------------------

Thanks Zhe for the update. Some comments.

1. Looking at some changes like below, I'm wondering if we could have any 
abstract concept for the two cases, replication and erasure coding or recovery. 
With that, something like {{neededReplications}} can be better named. However 
I'm not sure how much change will be incurred. You might be thinking "recovery" 
can cover "replication", but "recovery" is mainly discussed or mentioned and 
relevant in ec context and this issue title.
{code}
-   * Scan blocks in {@link #neededReplications} and assign replication
-   * work to data-nodes they belong to.
+   * Scan blocks in {@link #neededReplications} and assign recovery
+   * (replication or erasure coding) work to data-nodes they belong to.
...
-  int computeReplicationWork(int blocksToProcess) {
+  int computeBlockRecoveryWork(int blocksToProcess) {
{code}
2. Why we have this ? Are you using Java 8 ?
{code}
-            containingNodes = new ArrayList<DatanodeDescriptor>();
-            List<DatanodeStorageInfo> liveReplicaNodes = new 
ArrayList<DatanodeStorageInfo>();
+            containingNodes = new ArrayList<>();
+            List<DatanodeStorageInfo> liveReplicaNodes = new ArrayList<>();
{code}
3. In the codes for above, could we have initial capacity value for the two 
array lists ?
4. A minor, clean up.
{code}
+//              ErasureCodingWork ecw =
{code}
5. Assume we have conclusion about ec policy or unified storage policy in 
HDFS-7285, we may have new issue for the following other than HDFS-7337 (which 
mainly focuses on support of multiple codecs). Would we have one ?
{code}
// TODO: move erasure coding policy to file XAttr (HDFS-7337)
{code}
6. Better rename {{BlockCodecInfo}} since the system has many codec concepts.
7. In the test testMissingStripedBlock:
{code}
+    assertTrue("There should be 4 outstanding EC tasks", cnt > 0);
{code}
1) Can 4 be a variable like {{expectedRecoveryTasks}} to be calculated with 
constant value ?
2) Then, have {{cnt == expectedRecoveryTasks}} ?
8. Regarding below, I haven't better idea, and it looks good to me. Maybe we 
have some comments there for future optimization ? 
bq.how to efficiently get the indices of missing blocks. Maybe something like 
the below?


> Erasure coding: distribute recovery work for striped blocks to DataNode
> -----------------------------------------------------------------------
>
>                 Key: HDFS-7369
>                 URL: https://issues.apache.org/jira/browse/HDFS-7369
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Zhe Zhang
>            Assignee: Zhe Zhang
>         Attachments: HDFS-7369-000-part1.patch, HDFS-7369-000-part2.patch, 
> HDFS-7369-001.patch, HDFS-7369-002.patch
>
>
> This JIRA updates NameNode to handle background / offline recovery of erasure 
> coded blocks. It includes 2 parts:
> # Extend {{UnderReplicatedBlocks}} to recognize EC blocks and insert them to 
> appropriate priority levels. 
> # Update {{ReplicationMonitor}} to distinguish block codec tasks and send a 
> new DataNode command.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to