[
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)
