Varun Saxena commented on YARN-3051:

[~zjshen], thanks for looking at the patch.
bq.  I'd like to recommend we don't support the binary relationship in the 
initial jira. My intial param means to filter the entities who contain the 
given metrics
OK. will make the change. Let's move out this code to YARN-3863
One more thing I have changed is limit of entities by default have been kept 
has 100 instead of 1000. 1000 seemed too many. Thoughts ?

bq. Why do we need JsonSetter for the data model objects?
While reading back JSON dump from file, this is required.

bq. Maybe Field is better to be the inner class of TimelineReader or 
TimelineEntity. Otherwise, the name a bit vague about what it represents.

bq. A more critical problem is that FS reader and writer are not integrated:
Was thinking of raising a new JIRA to integrate writer and reader 
implementations because its not directly related to Reader API JIRA. Will do so.

bq. It seems to be better to implement FileSystemTimelineStorageImpl that 
implements both TimelineReader and TimelineWriter. 
Thats a good suggestion.

bq. Currently, when updating an entity, a new entity json will be appended into 
the same file, but this reader impl assumes one entity per file
Ok, so the last entity entry should be the one returned ? Will check the writer 
side code and ask if any queries. While combining both FS writer and reader, we 
can decide the best possible option.

bq.  Is "getTimelineRecordFromJSON" required to be exposed.
Added in TimelineUtils because dumpTimelineRecordtoJSON used by FS Writer was 
also put in the same class. And the ObjectMapper used by both the methods is 
also initialized in that class. If we combine FS Writer and Reader into one 
class, probably can move both methods into that class. Isn't likely to be used 
outside FS implementation.

> [Storage abstraction] Create backing storage read interface for ATS readers
> ---------------------------------------------------------------------------
>                 Key: YARN-3051
>                 URL: https://issues.apache.org/jira/browse/YARN-3051
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Sangjin Lee
>            Assignee: Varun Saxena
>         Attachments: YARN-3051-YARN-2928.003.patch, 
> YARN-3051-YARN-2928.03.patch, YARN-3051-YARN-2928.04.patch, 
> YARN-3051-YARN-2928.05.patch, YARN-3051.Reader_API.patch, 
> YARN-3051.Reader_API_1.patch, YARN-3051.Reader_API_2.patch, 
> YARN-3051.Reader_API_3.patch, YARN-3051.Reader_API_4.patch, 
> YARN-3051.wip.02.YARN-2928.patch, YARN-3051.wip.patch, YARN-3051_temp.patch
> Per design in YARN-2928, create backing storage read interface that can be 
> implemented by multiple backing storage implementations.

This message was sent by Atlassian JIRA

Reply via email to