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

Vinayakumar B commented on HDFS-7339:
-------------------------------------

Thanks [~zhz] for the patch.

Here are few Nits and doubts

Nits:

1. BlockGroup#hashCode() and #equals() should include id.

2. BlockGroup#numBytes and generationStamp are not effectively set anywhere. I 
believe numBytes will be set at client side and later committed to NN. But 
genstamp should be generated initially at the namenode side itself.

3. BlockGroup Copy constructor need include numBytes and generationStamp ?

4. BlockGroupManager#namesystem is never used. Can be removed

5. Following log could be DEBUG. {code}LOG.info("Creating new block group " + 
bg);{code}

6. Following comment needs to be updated for 
{{SequentialBlockGroupIdGenerator}} 
{code}* Generate the next valid block group ID by incrementing the maximum block
 * ID allocated so far, starting at 2^30+1.{code}



Doubts:
1. BlockGroupManager#chooseNewGroupTargets rejects the allocation if min number 
of nodes selected is less than groupSize. Is there anyway to continue with less 
number of nodes? at least for Testing purpose.
Otherwise even for testing minimum functionality need to so many nodes.
Because whatever the blocks generated after striping+EC will have different 
blockIds (within blockgroup), so ideally multiple blocks of same blockgroup can 
fit in same datanode.
in deployment mode, we can mandate minimum nodes.


2. Where the below mentioned method of identifying blockGroup from blockId will 
be used?
{code} * Contiguous: {reserved block IDs | flag | block ID}
 * Striped: {reserved block IDs | flag | block group ID | index in group}
 *
 * Following n bits of reserved block IDs, The (n+1)th bit in an ID
 * distinguishes contiguous (0) and striped (1) blocks. For a striped block,
 * bits (n+2) to (64-m) represent the ID of its block group, while the last m
 * bits represent its index of the group. The value m is determined by the
 * maximum number of blocks in a group (MAX_BLOCKS_IN_GROUP).{code}

> Allocating and persisting block groups in NameNode
> --------------------------------------------------
>
>                 Key: HDFS-7339
>                 URL: https://issues.apache.org/jira/browse/HDFS-7339
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Zhe Zhang
>            Assignee: Zhe Zhang
>         Attachments: HDFS-7339-001.patch, HDFS-7339-002.patch, 
> HDFS-7339-003.patch, HDFS-7339-004.patch, HDFS-7339-005.patch, 
> Meta-striping.jpg, NN-stripping.jpg
>
>
> All erasure codec operations center around the concept of _block group_; they 
> are formed in initial encoding and looked up in recoveries and conversions. A 
> lightweight class {{BlockGroup}} is created to record the original and parity 
> blocks in a coding group, as well as a pointer to the codec schema (pluggable 
> codec schemas will be supported in HDFS-7337). With the striping layout, the 
> HDFS client needs to operate on all blocks in a {{BlockGroup}} concurrently. 
> Therefore we propose to extend a file’s inode to switch between _contiguous_ 
> and _striping_ modes, with the current mode recorded in a binary flag. An 
> array of BlockGroups (or BlockGroup IDs) is added, which remains empty for 
> “traditional” HDFS files with contiguous block layout.
> The NameNode creates and maintains {{BlockGroup}} instances through the new 
> {{ECManager}} component; the attached figure has an illustration of the 
> architecture. As a simple example, when a {_Striping+EC_} file is created and 
> written to, it will serve requests from the client to allocate new 
> {{BlockGroups}} and store them under the {{INodeFile}}. In the current phase, 
> {{BlockGroups}} are allocated both in initial online encoding and in the 
> conversion from replication to EC. {{ECManager}} also facilitates the lookup 
> of {{BlockGroup}} information for block recovery work.



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

Reply via email to