Li Lu commented on YARN-4224:

Thanks for the work [~varun_saxena]! A few quick concerns:

1. I understand we would like to have plural forms for listing (like /flows, 
/apps) and singular forms for detail (like /flow/\{uid\}). But then, why do we 
need both /runs/\{uid\} and /run/\{uid\}? The same question also applies to 
2. For endpoints with UIDs, we need to work with flow, flowrun, app, and 
entity. I notice we have such code for flowrun (run) and app. For entities, we 
require both UID and type. Why type is not a part of UID (which means UID is 
not sufficient to identify an entity)? Or, are you planning to support 
operations like "list all entities in a given entity type"? If it is the 
latter, then do we want to consider put type into query parameters on end point 
entities? For flows, why we' re not including an UID endpoint to locate one 
flow? This poses a challenge when we'd like to list all flow runs within one 
flow (or, do we have any other end points to do this work? ). 
3. Seems like there is no full path to locate one entity from the cluster, 
user, flow, run, app, entity type, and entity id. Are we omitting this endpoint 
4. As a side note, in this patch there are 3 types of "shortcuts" in the URL: 
omit the cluster id (with default cluster id), omit user id (with default user 
id) and directly access app id. I'm OK with direct accessing app ids (with 
cluster id), but do we want to omit the other two? Comments are more then 

bq. 3. We have 2 options. Either set UID in TimelineReaderManager or in the 
storage implementation . Advantage of former is that we are delinking UID 
implementation from backend storage implementation. Disadvantage is that we 
need to iterate over all the entities again to set UID.
If we choose latter, it is the reverse. We can set UID while creating entities. 
But any new storage implementation needs to take care of filling UID then.
I have as of now implemented the second option. Not yet added that UID needs to 
be filled in javadoc.
bq. 4. Also UID is being returned as of now in both UID endpoint queries and 
non UID endpoint queries. Send UID only for former ?

I'm also debating with myself on this. Right now I'm leaning towards to make 
the UIDs transparent to the storage layer. Since UIDs will be added as an info 
field, it's more like an attachment to the original entities, but not a part of 
them. This can also keep writers easy (enforcing writers to add some data to 
all written entities looks a little awkward? ). Thoughts? 

> Change the ATSv2 reader side REST interface to conform to current REST APIs' 
> in YARN
> ------------------------------------------------------------------------------------
>                 Key: YARN-4224
>                 URL: https://issues.apache.org/jira/browse/YARN-4224
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Varun Saxena
>            Assignee: Varun Saxena
>              Labels: yarn-2928-1st-milestone
>         Attachments: YARN-4224-YARN-2928.01.patch, 
> YARN-4224-feature-YARN-2928.wip.02.patch

This message was sent by Atlassian JIRA

Reply via email to