Junping Du commented on YARN-3046:

Thanks [~rkanter] for review and comments!
bq. I think something that really hasn't been properly discussed is how 
compatible the ATS v1 APIs are with the v2 APIs, or how aware the user needs to 
be about which ATS they are using. (We're running into that in the other 
direction in YARN-2423).
Thanks for pointing out YARN-2423 which I already watched for a while. I agree 
that this is very important to discuss our compatibility story between v1 and 
v2 timeline services. However, given we already have NM and AM to leverage the 
same new TimelinetClient API for posting related new TimelineEntity, we can go 
through this way for MR in the short term. Given we already have a separated 
JIRA to track this: YARN-3049, may be we should go ahead to start some 
discussions from there?

bq. This isn't a GET call, so it should just say "putEntityNonBlocking failed: 
" + e
Sounds good. Will update it soon.

bq. Why not set the TimelineEvent Id in the toTimelineEvent() methods (or a 
parent method)?
Nice catch! Will update it soon.

bq. Given that all of the toTimelineEvent() methods are copy-paste of the 
switch statement in JobHistoryEventHandler#processEventForTimelineServer(), can 
you refactor processEventForTimelineServer() to use the toTimelineEvent() 
methods instead? That's a cleaner way of doing this and it's getting the same 
information. I know this is used by ATS v1, but we can still clean it up; 
especially given that it's all copy-pasted here anyway.
I had the same thoughts when I was starting the effort last week. However, I 
had a quick conversation offline with other guys, like [~vinodkv] and [~zjshen] 
and get buy-in with an opinion that ATS v1 on MR metrics is a released 
production and there could be some ongoing effort (bug fix, etc.) could happen 
in parallel on trunk and branch-2. Doing such a refactor on major piece could 
bring more risk of conflict or even involving with new bug which is not 
necessary for current phase. A better plan sounds like we do this refactor work 
after phase 2 (end-2-end can work properly) when we are merging back YARN-2928 
to trunk and branch-2. What do you think? If you also agree, I can file a 
separated JIRA against trunk to do the refactor work you mentioned here after 
we merge back. Thoughts?

> [Event producers] Implement MapReduce AM writing some MR metrics to ATS
> -----------------------------------------------------------------------
>                 Key: YARN-3046
>                 URL: https://issues.apache.org/jira/browse/YARN-3046
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Sangjin Lee
>            Assignee: Junping Du
>         Attachments: YARN-3046-no-test-v2.patch, YARN-3046-no-test.patch, 
> YARN-3046-v1-rebase.patch, YARN-3046-v1.patch
> Per design in YARN-2928, select a handful of MR metrics (e.g. HDFS bytes 
> written) and have the MR AM write the framework-specific metrics to ATS.

This message was sent by Atlassian JIRA

Reply via email to