[
https://issues.apache.org/jira/browse/HBASE-18792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16441547#comment-16441547
]
Umesh Agashe commented on HBASE-18792:
--------------------------------------
Thanks for your review [~busbey].
{quote}VersionInfo is IA.Public, so any public methods become a part of our
supported API. Can we make this method private?
{quote}
Good catch, fixed. Made package private.
{quote}ooof. man that's rough. How about a comment before the if(va ==
VERY_LARGE_NUMBER) that says something like "va and vb must both be the same
and indicate that the version part is a String"?
{quote}
Added comment as you have suggested.
{quote}Also make va, vb, and VERY_LARGE_NUMBER all type Integer instead of int
and use Integer#compareTo(Integer) so that we're not going through a bunch of
autoboxing.
{quote}
We may have to go through autoboxing 2 times on first 2 lines below. If va, vb
and VERY_LARGE_NUMBER are changed to type Integer then we will have to go
through autoboxing in return statement one or two time even if one of the
version components is of type String.
{code:java}
int va = v1Comps[index] instanceof Integer ? (Integer)v1Comps[index] :
VERY_LARGE_NUMBER;
int vb = v2Comps[index] instanceof Integer ? (Integer)v2Comps[index] :
VERY_LARGE_NUMBER;
if (va != vb) {
return va - vb;
}
{code}
{quote}I think this is wrong? like version "2.0.0" should be after
"2.0.0-SNAPSHOT". it's also after "2.0.0-alpha-3" or "2.0.0-beta-1".
Can we expand the versions checked in TestVersionInfo to include a) some "same
major different minor", b) "same minor different maintenance", c) both of the
above, but SNAPSHOT, d) "-alpha" / "-beta"?
{quote}
This comment is on existing comparison logic and needs separate JIRA to track
it. Please see HBASE-20444.
I think the existing version comparison logic is generic and tries to follow
lexicographic comparison. Its not specific to HBase versions. As commented,
2.0.0 should be after 2.0.0-SNAPSHOT and 2.0.0-alpha-3 but it should be before
2.0.0-patchXXXX or 2.0.0.1. Similarly 2.0 should be before 2.0.1 As improving
comparison for HBase specific version strings and adding unit tests are not in
scope of hbck changes I have created separate JIRA.
{quote}nit: We should use something like commons-lang's StringUtils#isNumeric
to check this so that we're not relying on exceptions for control flow. on the
other hand, this is a standard Java idiom so no big deal if we keep it.
{quote}
Agree, as these hbck changes doesn't really focus on existing parsing/
comparison logic. This can be addressed with the newly created JIRA HBASE-20444.
Please see the new patch. Thanks!
> hbase-2 needs to defend against hbck operations
> -----------------------------------------------
>
> Key: HBASE-18792
> URL: https://issues.apache.org/jira/browse/HBASE-18792
> Project: HBase
> Issue Type: Task
> Components: hbck
> Reporter: stack
> Assignee: Umesh Agashe
> Priority: Blocker
> Fix For: 2.0.0
>
> Attachments: hbase-18792.master.001.patch,
> hbase-18792.master.002.patch
>
>
> hbck needs updating to run against hbase2. Meantime, if an hbck from hbase1
> is run against hbck2, it may do damage. hbase2 should defend itself against
> hbck1 ops.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
