hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anu Engineer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-10268) Ozone: end-to-end integration for create/get volumes, buckets and keys.
Date Thu, 07 Apr 2016 05:19:25 GMT

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

Anu Engineer commented on HDFS-10268:
-------------------------------------

[~cnauroth] Thanks for contributing this patch. The code changes look very good, Especially
the streams code for input and output.

Some very minor comments:
# Nit: Not really something from your code, but I  just noticed this. 
In {{DataNode.java#Shutdown}} could you please move 
{code}
if(this.ozoneEnabled) {
      if(ozoneServer != null) {
        try {
          ozoneServer.stop();
        } catch (Exception e) {
          LOG.error("Error is ozone shutdown. ex {}", e.toString());
        }
      }
    }
  {code}
to after we stop the ObjectHandler ? 
{code}
 // Stop the object store handler
    if (this.objectStoreHandler != null) {
      this.objectStoreHandler.close();
    }
 {code}
 The logic being that it would avoid a race condition.
 # Not sure if this is valid. Wanted to flag it for your attention.
 In {{ObjectStoreHandler.java}} 
{code}
 storageHandler = new DistributedStorageHandler(new OzoneConfiguration(),
          this.storageContainerLocationClient);
{code}
Did you intend to allocate a new OzoneConfiguration or did you want to pass the config that
is in the constructor of ObjectStoreHandler. 
# The {{ObjectStoreHandler#close}} function might need to setup a flag or remove the storageHandler
function from {{ObjectStoreJerseyContainer}}. Please feel free to do this in a follow up JIRA
since it does not change the current behaviour. Looks like we might want to do another JIRA
for the proper shutdown sequencing.
# Just to confirm -- {{initSingleContainerPipeline}} is sync and not async, as comments seems
to indicate.
# in {{initSingleContainerPipeline}} {code} singlePipeline = newPipeline;{code} did you mean
to write this before finally ?
# Nit: in {{newPipelineFromNodes}}, we ignore the containerName generated in {{initSingleContainerPipeline}},
doesn't really matter since both are calls to UUID.random  
# Nit: {{OzoneContainerTranslation#fromContainerKeyValueListToBucket}} - It seems to read
the volumeName and BucketName from the arguments, it works, but just wanted to point out that
getKey does have those Values in the Metadata since you are setting it in {{OzoneContainerTranslation#fromBucketToContainerKeyData}}.
I can see that it is easier to read from Args, may be just leave a comment saying that it
is easier to read from Args than from KeyData ? 


> Ozone: end-to-end integration for create/get volumes, buckets and keys.
> -----------------------------------------------------------------------
>
>                 Key: HDFS-10268
>                 URL: https://issues.apache.org/jira/browse/HDFS-10268
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ozone
>            Reporter: Chris Nauroth
>            Assignee: Chris Nauroth
>         Attachments: HDFS-10268-HDFS-7240.001.patch
>
>
> The HDFS-7240 feature branch now has the building blocks required to enable end-to-end
functionality and testing for create/get volumes, buckets and keys.  The scope of this patch
is to complete the necessary integration in {{DistributedStorageHandler}} and related classes.



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

Mime
View raw message