[
https://issues.apache.org/jira/browse/HDFS-12063?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16068839#comment-16068839
]
Chen Liang edited comment on HDFS-12063 at 6/29/17 7:40 PM:
------------------------------------------------------------
Thanks [~linyiqun] for working on this! v001 LGTM overall, with some comments:
1. In the original code
{code}
OzoneVolume vol = client.getVolume(volumeName);
OzoneBucket bucket = vol.getBucket(bucketName);
bucket.putKey(keyName, dataFile);
{code}
in {{getVolume()}} and {{getBucket()}}, there are calls
{{OzoneUtils.verifyBucketName(volumeName);}} and
{{OzoneUtils.verifyBucketName(bucketName);}} which verifies the format of
volume name and bucket name, could you please also add them to the them
accordingly? (e.g. in new {{putKey()}}, after checking bucket name is not null,
call verifyBucketName to see if it's valid.) Also, although the function is
called verifyBucketName but it's also used to verify volume names. Please feel
free to rename this method if you want, it's up to you.
2. In new {{putKey()}}, there is {{IOUtils.closeStream(fis);}} I'm not sure
about the detail in {{.closeStream()}} , do need to we check fis is null or not
before calling it?
3. One thing about the original three RPC call is that, if the volume/bucket
does not exist, it throws exception, but the exceptions were somewhat
confusing, that's what led us to filing this JIRA in the beginning. So could
you please add unit test to see the behaviour of new put and get? e.g. try to
put/get from non-existing volumes/buckets and see if it does throw exceptions
as expected?
was (Author: vagarychen):
Thanks [~linyiqun] for working on this! v001 LGTM overall, with some comments:
1. In the original code
{code}
OzoneVolume vol = client.getVolume(volumeName);
OzoneBucket bucket = vol.getBucket(bucketName);
bucket.putKey(keyName, dataFile);
{code}
in {{getVolume()}} and {{getBucket()}}, there are calls
{{OzoneUtils.verifyBucketName(volumeName);}} and
{{OzoneUtils.verifyBucketName(bucketName);}} which verifies the format of
volume name and bucket name, could you please also add them to the them
accordingly? (e.g. in new {{putKey()}}, after checking bucket name is not null,
call verifyBucketName to see if it's valid.) Also, although the function is
called verifyBucketName but it's also used to verify volume names. Please feel
free to rename this method if you want, it's up to you.
2. In new {{putKey()}}, there is {{IOUtils.closeStream(fis);}} I'm not sure
about the detail in {{.closeStream()}} , do need to we check fis is null or not
before calling it?
3. One thing about the original three RPC call is that, I think if the volume
does not exist, it throws exception. So could you please add unit test to see
the behaviour of new put and get? e.g. try to put/get from non-existing
volumes/buckets and see if it does throw exceptions as expected?
> Ozone: Ozone shell: Multiple RPC calls for put/get key
> ------------------------------------------------------
>
> Key: HDFS-12063
> URL: https://issues.apache.org/jira/browse/HDFS-12063
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Nandakumar
> Assignee: Yiqun Lin
> Attachments: HDFS-12063-HDFS-7240.001.patch
>
>
> With current implementation multiple RPC calls are made for each put/get key
> ozone shell call
> {code:title=org.apache.hadoop.ozone.web.ozShell.keys.PutKeyHandler#execute}
> OzoneVolume vol = client.getVolume(volumeName);
> OzoneBucket bucket = vol.getBucket(bucketName);
> bucket.putKey(keyName, dataFile);
> {code}
> {code:title=org.apache.hadoop.ozone.web.ozShell.keys.GetKeyHandler#execute}
> OzoneVolume vol = client.getVolume(volumeName);
> OzoneBucket bucket = vol.getBucket(bucketName);
> bucket.getKey(keyName, dataFilePath);
> {code}
> This can be optimized.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
