[
https://issues.apache.org/jira/browse/HBASE-19920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16365624#comment-16365624
]
Sean Busbey commented on HBASE-19920:
-------------------------------------
I don't want to go down a rabbit hole with a late review, so if [~toffer] wants
to jump back in here, feel free to take the rest of this with a grain of salt.
nit: Having two classes with the same name and different packages is a code
smell for me. In logging contexts plenty of things try to "cleverly" shorten
names and drop package names, especially in the middle of deep package nesting.
ProtobufUtil and ProtobufExceptionUtil both seem likely to get this treatment,
which will be confusing for debugging later. The notes in ProtobufUtil
certainly help, but is there a big disadvantage to naming one e.g.
NonShadedProtobufUtil?
Given the guidance about keeping the two ProtobufUtil implementations in sync,
notes about why they currently differ would help. For example, the *toScan*
methods in the non-shaded version skip the stanzas around handling
*NeedCursorResult* in both directions.
nit: the DeserializationException construction in *expectPBMagicPrefix* in
shaded.ProtobufUtil is different from the non-shaded version and does an extra
concatenation of string literals.
Looking at the two *ProtobufExceptionUtil.java* implementations in here, they
should have similar notices to those in ProtobufUtil and I have the same
concerns about overlapping class names. Maybe in this case the non-shaded
version could leverage the shaded one since it already references both versions
of ServiceException?
> TokenUtil.obtainToken unnecessarily creates a local directory
> -------------------------------------------------------------
>
> Key: HBASE-19920
> URL: https://issues.apache.org/jira/browse/HBASE-19920
> Project: HBase
> Issue Type: Bug
> Reporter: Rohini Palaniswamy
> Assignee: Mike Drob
> Priority: Major
> Fix For: 2.0
>
> Attachments: HBASE-19920.patch, HBASE-19920.v2.patch,
> HBASE-19920.v3.patch, HBASE-19920.v4.patch, HBASE-19920.v5.patch,
> HBASE-19920.v6.patch, HBASE-19920.v7.patch, HBASE-19920.v8.patch
>
>
> On client code, when one calls TokenUtil.obtainToken it loads ProtobufUtil
> which in its static block initializes DynamicClassLoader and that creates the
> directory ${hbase.local.dir}/jars/ and also instantiates a filesystem class
> to access hbase.dynamic.jars.dir.
> https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/util/DynamicClassLoader.java#L109-L127
> Since this is region server specific code, not expecting this to happen when
> one accesses hbase as a client.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
