[
https://issues.apache.org/jira/browse/HDFS-12549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16236752#comment-16236752
]
Xiaoyu Yao edited comment on HDFS-12549 at 11/2/17 11:49 PM:
-------------------------------------------------------------
Thanks [~nandakumar131] for working on this. The patch looks good overall to
me. Just a few suggestions:
Please separate the broad refactoring change such as class/package rename in a
separate JIRAs so that we can focus on reviewing core changes for this JIRA.
OzoneClientUtils.java
Line 108-115, 847-857: have you consider using the new JDK8
DateTimeFormatter/ZonedDateTime for the handling of ozone creation/modification
timestamp?
OzoneConfigKeys.java
This is related to the polling http client for ozone rest client. Please add
some comments to it and the default values for
OZONE_REST_CLIENT_HTTP_CONNECTION_MAX and maybe some document also in
ozone-default.xml
RestClient.java
If I understand correctly, this class holds the major change for this JIRA.
Line 135: there are other configurations of PoolingHttpClientConnectionManager
that we might want to expose via OzoneConfigKeys in addition to the MaxTotal,
e.g., max per route which by default is only 2. Considering lots of client will
try to connect to the same REST server port, the effectiveness of the
connection pool might be limited for ozone with some of the default values.
Line 150: we don't need to instantiate a new data formatter here. The hard
coded format string can be replaced by OzoneConsts.OZONE_DATE_FORMAT.
I would also suggest we either use the instance from OzoneClientUtils or use
the New JDK8 DataTimeFormatter/Instance, etc.
Line 706: executeHttpRequest does not close the response, which causes leaking
of the response stream. I would suggest we put EntityUtils.consume inside
executeHttpRequest so that the connection will be released after consumption
and the response will be closed after that like below.
{code}
CloseableHttpResponse response = httpclient.execute(httpUriRequest);
try {
…
EntityUtils.consume(entity1);
} finally {
response.close();
}
{code}
Simple.java
Line 113: should use the getShortUserName().
currentUser=UserGroupInformation.getCurrentUser().getShortUserName();
was (Author: xyao):
Thanks [~nandakumar131] for working on this. The patch looks good overall to
me. Just a few suggestions:
Please separate the broad refactoring change such as class/package rename in a
separate JIRAs so that we can focus on reviewing core changes for this JIRA.
OzoneClientUtils.java
Line 108-115, 847-857: have you consider using the new JDK8
DateTimeFormatter/ZonedDateTime for the handling of ozone creation/modification
timestamp?
OzoneConfigKeys.java
This is related to the polling http client for ozone rest client. Please add
some comments to it and the default values for
OZONE_REST_CLIENT_HTTP_CONNECTION_MAX and maybe some document also in
ozone-default.xml
RestClient.java
If I understand correctly, this class holds the major change for this JIRA.
Line 135: there are other configurations of PoolingHttpClientConnectionManager
that we might want to expose
Via OzoneConfigKeys in addition to the MaxTotal, e.g., max per route which by
default is only 2. Considering lots of client will try to connect to the same
REST server port, the effectiveness of the connection pool might be limited for
ozone with some of the default values.
Line 150: we don't need to instantiate a new data formatter here. The hard
coded format string can be replaced by OzoneConsts.OZONE_DATE_FORMAT.
I would also suggest we either use the instance from OzoneClientUtils or use
the New JDK8 DataTimeFormatter/Instance, etc.
Line 706: executeHttpRequest does not close the response, which causes leaking
of the stream.
We should put EntityUtils.consume inside executeHttpRequest and close the
response after entity is consumed.
{code}
CloseableHttpResponse response = httpclient.execute(httpUriRequest);
try {
…
EntityUtils.consume(entity1);
} finally {
response.close();
}
{code}
Simple.java
Line 113: should use the getShortUserName().
currentUser=UserGroupInformation.getCurrentUser().getShortUserName();
> Ozone: OzoneClient: Support for REST protocol
> ---------------------------------------------
>
> Key: HDFS-12549
> URL: https://issues.apache.org/jira/browse/HDFS-12549
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Reporter: Nanda kumar
> Assignee: Nanda kumar
> Priority: Major
> Attachments: HDFS-12549-HDFS-7240.000.patch,
> HDFS-12549-HDFS-7240.001.patch, HDFS-12549-HDFS-7240.002.patch,
> HDFS-12549-HDFS-7240.003.patch
>
>
> Support for REST protocol in OzoneClient.
--
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
