[ https://issues.apache.org/jira/browse/YARN-5170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15323631#comment-15323631 ]
Joep Rottinghuis commented on YARN-5170: ---------------------------------------- Thanks for the review [~gtCarrera9] bq. In HBaseTimelineWriterImpl isApplicationEntity and getApplicationEvent seem to be awkward. Looks more like something related to TimelineEntity or ApplicationEntity Agreed. Are you suggesting to move this to: org.apache.hadoop.yarn.api.records.timelineservice.ApplicationEntity When I look at the call history, this (1 line) method is used just in HBaseTimelineWriterImpl. Rather than a larger refactor now, shall I just make this a private method? > Eliminate singleton converters and static method access > ------------------------------------------------------- > > Key: YARN-5170 > URL: https://issues.apache.org/jira/browse/YARN-5170 > 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-5170-YARN-2928.01.patch, > YARN-5170-YARN-2928.02.patch, YARN-5170-YARN-2928.03.patch, > YARN-5170-YARN-2928.04.patch, YARN-5170-YARN-2928.05.patch, > YARN-5170-YARN-2928.06.patch, YARN-5170-YARN-2928.07.patch, > YARN-5170-YARN-2928.08.patch, YARN-5170-YARN-2928.09.patch, > YARN-5170-YARN-2928.10.patch > > > As part of YARN-5109 we introduced several KeyConverter classes. > To stay consistent with the existing LongConverter in the sample patch I > created I made these other converter classes singleton as well. > In conversation with [~sjlee0] who has a general dislike of singletons, we > discussed it is best to get rid of these singletons and make them simply > instance variables. > There are other classes where the keys have static methods referring to a > singleton converter. > Moreover, it turns out that due to code evolution we end up creating the same > keys several times. > So general approach is to not re-instantiate rowkeys, converters when not > needed. > I would like to create the byte[] rowKey in the RowKey classes their > constructor, but that would leak an incomplete object to the converter. > There are a few method in TimelineStorageUtils that are used only once, or > only by one class, as part of this refactor I'll move these to keep the > "Utils" class as small as possible and keep them for truly generally used > utils that don't really belong anywhere else. -- 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