[ 
https://issues.apache.org/jira/browse/HBASE-5959?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13273744#comment-13273744
 ] 

stack commented on HBASE-5959:
------------------------------

The move of LoadBalancer down into a package is a no-no given its a Public 
Interface (see the annotation just above class declaration).  Would need to go 
through a deprecation cycle.   Factory can go down into the new balancer 
package.  Thats fine.

You don't need these lines:

{code}
+ * Copyright 2012 The Apache Software Foundation
{code}

Is BalanceInfo generically useful or is it useful only to your new balance algo?

Fix 'It provides the the functions used to by'

You don't want to pass in master services on construction?  You don't want to 
pass cluster status on invocation of the balance function?  Rather than use 
setters (when setters, no insurance when they will be called)

Need class comment on new class ClusterLoadState.  Does it need to be public?  
Do methods in this class need to be public?  Can they be package protected?

Did you just move the old classes or do you do some changes to them (I did not 
review for changes).


RegionInfoComparator seems like a misleading name given what it does?  Indent 
is not the two spaces used in rest of code base.

How often will we run this block location calcuation?  Its expensive.  We need 
to be careful.

RegionPlanCompartor should be inner class of RegionPlan?

Will do a deeper review later but so far, looks great (need to look at your new 
doohickey balancer more)


                
> Add other load balancers
> ------------------------
>
>                 Key: HBASE-5959
>                 URL: https://issues.apache.org/jira/browse/HBASE-5959
>             Project: HBase
>          Issue Type: New Feature
>          Components: master
>    Affects Versions: 0.96.0
>            Reporter: Elliott Clark
>            Assignee: Elliott Clark
>         Attachments: HBASE-5959-0.patch, HBASE-5959-1.patch, 
> HBASE-5959-2.patch, HBASE-5959-3.patch
>
>
> Now that balancers are pluggable we should give some options.b

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to