[ https://issues.apache.org/jira/browse/YARN-5189?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15311807#comment-15311807 ]
Sangjin Lee commented on YARN-5189: ----------------------------------- On the patch itself, I found several more places where we need to set the reader/writer explicitly. (TestTimelineReaderServer.java) - l.41: it should set the reader class, not the writer (TestSystemMetricsPublisherForV2.java) - we should set the writer class to {{FileSystemTimelineWriterImpl}} here (TestMRTimelineEventHandling.java) - we should set the writer class to {{FileSystemTimelineWriterImpl}} here (TestTimelineServiceClientIntegration.java) - we also should set the writer class to {{FileSystemTimelineWriterImpl}} here - I noticed that it throws NPE even with the filesystem store, and it turns out to be an existing issue. It's because not all the context is provided to the timeline collector manager. In {{MockNodeTimelineCollectorManager.getNMCollectorService()}}, the following should be done: {code} GetTimelineCollectorContextResponse response = GetTimelineCollectorContextResponse.newInstance( UserGroupInformation.getCurrentUser().getShortUserName(), "test_flow_name", "test_flow_version", 1L); {code} (TestDistributedShell.java) - l.185: spell out the package name for {{TimelineWriter}} explcitly as there are 2 {{TimelineWriter}} types in this test; here it should be {{org.apache.hadoop.yarn.server.timelineservice.storage.TimelineWriter.class}}; otherwise the test fails (TestTimelineReaderWebServices.java) - l.113: why not simply use {{assertEquals()}}? > Make HBaseTimeline[Reader|Writer]Impl default and move FileSystemTimeline*Impl > ------------------------------------------------------------------------------ > > Key: YARN-5189 > URL: https://issues.apache.org/jira/browse/YARN-5189 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver > Affects Versions: YARN-2928 > Reporter: Joep Rottinghuis > Assignee: Joep Rottinghuis > Labels: yarn-2928-1st-milestone > Attachments: YARN-5189-YARN-2928.01.patch, > YARN-5189-YARN-2928.02.patch > > > [~naganarasimha...@apache.org] questioned whether it made sense to default to > an implementation that doesn't support all functionality. > [~sjlee0] opened YARN-5174 to track updating the documentation for ATS to > reflect the default shifting to the fully functional HBase implementation. > It makes sense to remove a partial implementation, but on the other hand it > is still handing in testing. Hence this jira to move the file based > implementations to the test package and to make the HBase impls the default. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org