[
https://issues.apache.org/jira/browse/YARN-5260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Joep Rottinghuis updated YARN-5260:
-----------------------------------
Labels: YARN-5355 (was: )
> 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
> Labels: YARN-5355
>
> [~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
