[ https://issues.apache.org/jira/browse/YARN-5260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15339965#comment-15339965 ]
Joep Rottinghuis commented on YARN-5260: ---------------------------------------- InternalScanner is now tagged with LimitedPrivate(HBaseInterfaceAudience.COPROC) as of HBase 1.3.0 and up as per HBASE-16048 > Review / Recommendations for hbase writer code > ---------------------------------------------- > > Key: YARN-5260 > URL: https://issues.apache.org/jira/browse/YARN-5260 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver > Reporter: Vrushali C > Assignee: Vrushali C > > [~ted_yu] is graciously reviewing the hbase writer related code and has some > recommendations. (more to come as review progresses). I will keep track of > those in this jira and perhaps spin off other jira(s) depending on the scope > of changes. > For FlowRunCoprocessor.java : > > - private HRegion region; > Try to declare as Region - the interface. This way, you are to call methods > that are stable across future releases. > - private long getCellTimestamp(long timestamp, List<Tag> tags) { > tags is not used, remove the parameter. > For FlowScanner: > - private final InternalScanner flowRunScanner; > Currently InternalScanner is Private. If you must use it, try surfacing your > case to hbase so that it can be marked: > @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.COPROC) > @InterfaceStability.Evolving > w.r.t. regionScanner : > {code} > if (internalScanner instanceof RegionScanner) { > this.regionScanner = (RegionScanner) internalScanner; > } > {code} > I see IllegalStateException being thrown in some methods when regionScanner > is null. Better bail out early in the ctor. > {code} > public static AggregationOperation getAggregationOperationFromTagsList( > List<Tag> tags) { > for (AggregationOperation aggOp : AggregationOperation.values()) { > for (Tag tag : tags) { > if (tag.getType() == aggOp.getTagType()) { > return aggOp; > {code} > The above nested loop can be improved (a lot): > values() returns an array. If you pre-generate a Set > (https://docs.oracle.com/javase/7/docs/api/java/util/EnumSet.html) containing > all the values, the outer loop can be omitted. > You iterate through tags and see if tag.getType() is in the Set. -- 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