hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Junping Du (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-2302) Refactor TimelineWebServices
Date Fri, 08 Aug 2014 12:32:13 GMT

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

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

Thanks for the patch, [~zjshen]! A couple of comments so far:

In ApplicationHistoryServer.java,

{code}
+  protected TimelineDataManager timelineDataManager;
{code}
Better to be private, as it only get consumed within ApplicationHistoryServer.

{code}
    timelineACLsManager = createTimelineACLsManager(conf);
{code}
Looks like we don’t need timelineACLsManager anymore except initiating TimeLineDataManager.
We can completely remove it (method and variable) after merge below
{code}
  protected TimelineACLsManager createTimelineACLsManager(Configuration conf) {
    return new TimelineACLsManager(conf);
  }

  protected TimelineDataManager createTimelineDataManager(Configuration conf) {
    return new TimelineDataManager(timelineStore, timelineACLsManager);
  }
{code}

to:

{code}
  private TimelineDataManager createTimelineDataManager(Configuration conf) {
    return new TimelineDataManager(timelineStore, new TimelineACLsManager(conf));
  }
{code}
The visibility of method should be private, as it is not get assumed outside of class. There
are also some similar unnecessary protected methods around in this class, see if you want
to do update here also or we can do it separately later.

In TimelineDataManager.java,
{code}
+      try {
+        if (existingEntity == null) {
+          injectOwnerInfo(entity, callerUGI.getShortUserName());
+        }
+      } catch (YarnException e) {
+        // Skip the entity which messes up the primary filter and record the
+        // error
+        LOG.warn("Skip the timeline entity: " + entityID + ", because "
+            + e.getMessage());
{code}
This exception sounds more serious than just a warn, so Log.error here may make more sense?

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

Mime
View raw message