[ 
https://issues.apache.org/jira/browse/HDFS-10268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chris Nauroth updated HDFS-10268:
---------------------------------
    Attachment: HDFS-10268-HDFS-7240.002.patch

[~anu], thank you for the helpful review.  I am attaching patch v002.

Good point on the DataNode shutdown sequence.  I made the change.  Also 
important is that both of these are stopped after the HTTP server is stopped, 
so new incoming HTTP requests won't accidentally get dispatched to stopped 
handler objects.

bq. Did you intend to allocate a new OzoneConfiguration or did you want to pass 
the config that is in the constructor of ObjectStoreHandler?

The object passed to the constructor is a plain {{Configuration}}, but 
{{DistributedStorageHandler}} needs an {{OzoneConfiguration}}.  I agree that 
it's awkward and sub-optimal that we have to deal with both, so we might want 
to look for a better way in a separate follow-up patch.

bq. The {{ObjectStoreHandler#close}} function might need to setup a flag or 
remove the storageHandler function from {{ObjectStoreJerseyContainer}}.

As per comment 1, your suggested change in the shutdown sequence makes it so 
that the HTTP server is stopped before the {{ObjectStoreHandler}}.  Therefore, 
there are no new incoming HTTP requests that would get dispatched to 
{{ObjectStoreJerseyContainer}} by the time we are calling 
{{ObjectStoreHandler#close}}.  I think this is OK, but we definitely can do 
follow-up patches if you still see an issue.

bq. Just to confirm – {{initSingleContainerPipeline}} is sync and not async, as 
comments seems to indicate.

Yes, this is synchronous.  I tried to clarify this in the comments.

bq. did you mean to write this before finally ?

I don't think it makes a difference to the logic, but I do think it's more 
readable to put {{singlePipeline = newPipeline;}} as the last statement of the 
try block.  I made the change.

bq. we ignore the containerName generated in {{initSingleContainerPipeline}}

Oops!  Thanks for catching that.

bq. ...just wanted to point out that getKey does have those Values in the 
Metadata...

That's a great point.  We really don't need the web request args here.  I think 
that's a relic from earlier stub code when I wasn't fully persisting metadata 
and instead just echoing back the web request args.  I changed it to pull from 
the metadata instead, and I also made a similar change for 
{{fromContainerKeyValueListToVolume}}.


> 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, 
> HDFS-10268-HDFS-7240.002.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)

Reply via email to