[ https://issues.apache.org/jira/browse/YARN-3051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14606542#comment-14606542 ]
Zhijie Shen commented on YARN-3051: ----------------------------------- Varun, thanks for take over the reader interface patch. I noticed there's a param difference: Map<String, NameValueRelations> metricFilters. 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. We can file a separate jira to add binary relationship filtering, for metrics, config and info altogether. How do you think? Here's a couple of comments about the patch: 1. Why do we need JsonSetter for the data model objects? 2. Can we prevent introducing the test oriented configurations into YarnConfiguration, which is part of api? 3. Is "getTimelineRecordFromJSON" required to be exposed. I'm a bit conservative to put the methods in api/common, which mean we need to keep supporting it. 4. Maybe {{Field}} is better to be the inner class of TimelineReader or TimelineEntity. Otherwise, the name a bit vague about what it represents. 5. It seems to be better to implement FileSystemTimelineStorageImpl that implements both TimelineReader and TimelineWriter. One motivation is to reuse some code. A more critical problem is that FS reader and writer are not integrated: a) Writer should not have written the mapping into APP_FLOW_MAPPING_FILE. b) 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. We need to sync the behavior between them. > [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 (v6.3.4#6332)