hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joep Rottinghuis (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-5170) Eliminate singleton converters and static method access
Date Thu, 09 Jun 2016 22:15:24 GMT

    [ https://issues.apache.org/jira/browse/YARN-5170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15322919#comment-15322919

Joep Rottinghuis edited comment on YARN-5170 at 6/9/16 10:15 PM:

[~varun_saxena] Thank you for your review, this is great feedback, I'll work on this and upload
a new patch asap.

bq. We have readMetrics method in TimelineEntityReader. Should other read*** methods be moved
there as well ? Who knows in future we add a table which needs to read only KV pairs and hence
does not directly belong as a subclass to GenericEntityReader.
Moved most methods, but left #readKeyValuePairs(TimelineEntity entity, Result result,
      ColumnPrefix<T> prefix, boolean isConfig) in GenericEntityReader given the specificity
from isConfig.
I started to look to refactor entity.addInfo and entity.addConfig to make them more similar,
but I abandoned that part of the refactor to focus on all the other things that this patch
already does.
I think if we can make it more generic and not config specific, then we can certainly move
this method up the hierarchy.

was (Author: jrottinghuis):
[~varun_saxena] Thank you for your review, this is great feedback, I'll work on this and upload
a new patch asap.

> 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
> 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
> 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

To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org

View raw message