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-4234) New put APIs in TimelineClient for ats v1.5
Date Thu, 17 Dec 2015 16:39:46 GMT

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

Junping Du commented on YARN-4234:
----------------------------------

Thanks [~xgong] for updating the patch. I just go through the latest patch, and below is my
review comments:

In TimelineEntityGroupId.java,
Can we add Javadoc for TimelineEntityGroupId and explain what TimelineEntityGroupId is used
for?
Also in TimelineEntityGroupId.equals(), can we omit unnecessary ";"?

In FileSystemTimelineWriter.java,
{noformat}
      LOG.debug(
          YarnConfiguration.TIMELINE_SERVICE_CLIENT_FD_FLUSH_INTERVAL_SECS
              + "=" + flushIntervalSecs + ", " +
          YarnConfiguration.TIMELINE_SERVICE_CLIENT_FD_CLEAN_INTERVAL_SECS
              + "=" + cleanIntervalSecs + ", " +
          YarnConfiguration.TIMELINE_SERVICE_CLIENT_FD_RETAIN_SECS
              + "=" + ttl + ", " +
          TIMELINE_SERVICE_ENTITYFILE_FS_SUPPORT_APPEND
              + "=" + isAppendSupported);
{noformat}
Shall we log other related configurations, like: activePath, retryPolicy, summaryEntityTypes,
etc.?

In putEntities(),
The name of entitiesToSummary, entitiesToEntity and entitiesToDB sounds a little confusing.
How about update them to entitiesToSummaryCache, entitiesToEntityCache and entitiesToDBStore?
Also, we shouldn't log in INFO level for each entity get written in cache. Debug level should
be good.

In LogFDsCache,
{noformat}
      summanyLogFDs = new HashMap<ApplicationAttemptId, EntityLogFD>();
      entityLogFDs = new HashMap<ApplicationAttemptId,
          HashMap<TimelineEntityGroupId, EntityLogFD>>();
{noformat}
There is a race condition here: if LogFDsCache is doing flush with iterating the map and some
writing entity/summary come to insert the HashMap at the same time, then it will throw ConcurrentModificationException.
We should use ConcurrentHashMap instead.
Also, for CleanInActiveFDsTask, We should use WARN level for LOG instead of DEBUG in case
exception get throw.

In TimelineWriter.java,
The conf seems not get used. If so, we can remove it from class and constructor?

Other looks good to me.

> New put APIs in TimelineClient for ats v1.5
> -------------------------------------------
>
>                 Key: YARN-4234
>                 URL: https://issues.apache.org/jira/browse/YARN-4234
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-4234-2015-11-13.1.patch, YARN-4234-2015-11-16.1.patch, YARN-4234-2015-11-16.2.patch,
YARN-4234-20151111.2.patch, YARN-4234.1.patch, YARN-4234.2.patch, YARN-4234.2015-11-12.1.patch,
YARN-4234.2015-11-12.1.patch, YARN-4234.2015-11-18.1.patch, YARN-4234.2015-11-18.2.patch,
YARN-4234.2015-12-09.patch, YARN-4234.2015-12-09.patch, YARN-4234.20151109.patch, YARN-4234.20151110.1.patch,
YARN-4234.20151111.1.patch, YARN-4234.3.patch
>
>
> In this ticket, we will add new put APIs in timelineClient to let clients/applications
have the option to use ATS v1.5



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message