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

Junping Du commented on YARN-2302:
----------------------------------

Thanks for updating the patch, [~zjshen]! The patch looks good now in overall. 
Some minor comments:
{code}
+  public TimelinePutResponse postEntities(
+      TimelineEntities entities,
+      UserGroupInformation callerUGI) throws YarnException, IOException {
+    if (entities == null) {
{code}
Shall we rename this method to putEntities? There is slightly different between 
put and post operation (in REST prospective) while post is "create" (first 
time) but put is an "update". The internal behavior of the method is like 
"update" and call put operation actually, so "put" could be more properly.
In addition, I think we should have javadoc for public methods in 
TimelineDataManager.java. 
Other looks fine.

> Refactor TimelineWebServices
> ----------------------------
>
>                 Key: YARN-2302
>                 URL: https://issues.apache.org/jira/browse/YARN-2302
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Zhijie Shen
>            Assignee: Zhijie Shen
>         Attachments: YARN-2302.1.patch, YARN-2302.2.patch
>
>
> Now TimelineWebServices contains non-trivial logic to process the HTTP 
> requests, manipulate the data, check the access, and interact with the 
> timeline store.
> I propose the move the data-oriented logic to a middle layer (so called 
> TimelineDataManager), and TimelineWebServices only processes the requests, 
> and call TimelineDataManager to complete the remaining tasks.
> By doing this, we make the generic history module reuse TimelineDataManager 
> internally (YARN-2033), invoking the putting/getting methods directly. 
> Otherwise, we have to send the HTTP requests to TimelineWebServices to query 
> the generic history data, which is not an efficient way.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to