[ https://issues.apache.org/jira/browse/YARN-3904?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14648457#comment-14648457 ]
Zhijie Shen commented on YARN-3904: ----------------------------------- [~gtCarrera9], thanks for the patch. Bellow are my comments: bq. The two failed tests passed on my local machine, and the failures appeared to be irrelevant. This said, we may still need to fix those intermittent test failures. Do we plan to fix it in this patch? Some high level comments: 1. As is also mentioned in YARN-3049, how about we refactoring reader/writer method signature in a separate jira to avoid conflicts? 2. I suggest moving the table creation stuff into TimelineSchemaCreator. 3. As HBase backend is accessed both directly and via Phoenix, it's good for us to cleanup the configuration to say we're using the HBase backend (comparing to FS backend) instead of specifically HBase or Phoenix writer/reader. Other patch details: 1. Make OfflineAggregationWriter extend Service, such that you don't need to define init. 2. Now we're working towards a production standard patch. Would you please write some javadoc to explain the schema of the aggregation tables like what we did for HBase tables. 3. The connection config should be moved to YarnConfiguration. 4. Why is info column family kept? I expect the aggregation table will only have metrics data 5. Let's also have a default PhoenixOfflineAggregationWriterImpl constructor to be used in the production code. 6. {{Class.forName(DRIVER_CLASS_NAME);}} doesn't need to be invoked every time we get a connection. > Refactor timelineservice.storage to add support to online and offline > aggregation writers > ----------------------------------------------------------------------------------------- > > Key: YARN-3904 > URL: https://issues.apache.org/jira/browse/YARN-3904 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver > Reporter: Li Lu > Assignee: Li Lu > Attachments: YARN-3904-YARN-2928.001.patch, > YARN-3904-YARN-2928.002.patch, YARN-3904-YARN-2928.003.patch, > YARN-3904-YARN-2928.004.patch, YARN-3904-YARN-2928.005.patch > > > After we finished the design for time-based aggregation, we can adopt our > existing Phoenix storage into the storage of the aggregated data. In this > JIRA, I'm proposing to refactor writers to add support to aggregation > writers. Offline aggregation writers typically has less contextual > information. We can distinguish these writers by special naming. We can also > use CollectorContexts to model all contextual information and use it in our > writer interfaces. -- This message was sent by Atlassian JIRA (v6.3.4#6332)