Sangjin Lee commented on YARN-3041:

Apologies for not chiming in sooner. I have been held up by a number of things 
that are going on.

The patch looks mostly good. Thanks [~zjshen] for putting up the patch and the 
proposal. I also agree that we should get the patch committed soon and adjust 
later if necessary.

Having said that, I do have some quick high level comments, and hopefully they 
are mostly straightforward.

- In defining types, how about using enums (i.e. stronger types) as opposed to 
strings? Have you considered enums?
- In HierarchicalTimelineEntity.addChildren(), I think there is a bug. The 
*argument* "children" is being iterated upon, but within the for loop, it is 
accessed and mutated again. I'm pretty sure this will throw a 
ConcurrentModificationException. I think the intent is to interact with the 
*member variable* "children" inside the loop, right?
- Shouldn't TimelineUser and TimelineQueue extend TimelineEntity? They can (and 
will) have things like metrics associated with them.
- In what sense could the flow and the cluster be parents of the application 
entity? Are we talking about the “single application” situation outside the 
context of a flow? Especially, I’m not sure about the flow being a parent of an 
application. The flow is a *template*, and only a specific realization of a 
flow (i.e. flow run) can contain real applications. So I don't think a flow can 
be a direct parent of an application. Only a flow run can be one.

> [Data Model] create overall data objects of TS next gen
> -------------------------------------------------------
>                 Key: YARN-3041
>                 URL: https://issues.apache.org/jira/browse/YARN-3041
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Sangjin Lee
>            Assignee: Zhijie Shen
>         Attachments: Data_model_proposal_v2.pdf, YARN-3041.2.patch, 
> YARN-3041.3.patch, YARN-3041.preliminary.001.patch
> Per design in YARN-2928, create the ATS entity and events API.
> Also, as part of this JIRA, create YARN system entities (e.g. cluster, user, 
> flow, flow run, YARN app, ...).

This message was sent by Atlassian JIRA

Reply via email to