hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-8210) Ozone: Implement storage container manager
Date Mon, 18 May 2015 18:16:02 GMT

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

Chris Nauroth commented on HDFS-8210:

Hi Jitendra.  This is a great start.  The separation of responsibilities between {{StorageContainerMap}}
and {{BitWiseTrieContainerMap}} makes sense.  Here are a few comments.
# Generally, we could use more comments, especially regarding how we manage the keyspace and
represent that in the trie.  I defer to you on whether or not to add comments right now or
wait until a later date when the code is more settled.  We're on a feature branch, so we have
# {{StorageContainerMap#iterator}}: Please add a more descriptive message to the exception.
 Alternatively, does it make sense to go ahead and implement an iterator that iterates through
{{containerPrefixMap.values()}} and then traverses the {{TrieNode}} instances for each one?
# {{StorageContainerMap#initPrefix}}: I believe there is a thread-safety problem between calling
{{getTrieMap}} (lock held) followed by {{containerPrefixMap.put}} (lock not held).  Does this
whole method need to be {{synchronized}}?  Alternatively, maybe you could switch to a {{ConcurrentHashMap}}
and use {{putIfAbsent}}.  Also, I wonder if the API would be more consistent if the caller
could pass in the prefix in the upper bits of the argument (like {{getTrieMap}} and other
methods expect) and let this class encapsulate all of the bit manipulation.  This patch doesn't
yet call {{initPrefix}} anywhere, so maybe passing it in the lower bits will make more sense
when I see that.
# {{StorageContainerMap#splitContainer}}: The call to {{BitWiseTrieContainerMap#addBit}} is
thread-safe, but I wonder if the operation of splitting a container is going to need a stronger
guarantee of idempotence, so that multiple concurrent requests that need a split don't result
in multiple {{addBit}} calls.  This might give rise to an API more like {{ensureBits(int n)}},
with a post-condition that the bit length is at least n.
# {{BitWiseTrieContainerMap}}: The constructor hard-codes replication to 3.  Do you expect
to revisit this?  If so, would you mark a TODO?
# {{BitWiseTrieContainerMap#getInternal}}: I'm wary of recursive methods in Java, since we
don't get tail-call optimization.  However, in this case, I think it's fine, because we expect
the maximum depth of the trie to be small: 64 - {{prefixLength}} = 36.  Would you please add
a comment about this?
# {{TestStorageContainerMap}}: If our prefix length is 28, then we expect to be able to split
36 times, correct?  If so, would you please add a test that splits 36 times and then asserts
that the pre-condition check fails on the 37th split attempt?

> Ozone: Implement storage container manager 
> -------------------------------------------
>                 Key: HDFS-8210
>                 URL: https://issues.apache.org/jira/browse/HDFS-8210
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Jitendra Nath Pandey
>            Assignee: Jitendra Nath Pandey
>         Attachments: HDFS-8210-HDFS-7240.1.patch, HDFS-8210-HDFS-7240.2.patch
> The storage container manager collects datanode heartbeats, manages replication and exposes
API to lookup containers. This jira implements storage container manager by re-using the block
manager implementation in namenode. This jira provides initial implementation that works with
datanodes. The additional protocols will be added in subsequent jiras.

This message was sent by Atlassian JIRA

View raw message