[ https://issues.apache.org/jira/browse/HBASE-5959?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13285170#comment-13285170 ]
stack commented on HBASE-5959: ------------------------------ Why does RegionPlanComparator implemenent Serializable? Maybe doc why you are doing this compare on regionid... are you dependent on it being timestamp? Why not do 'return r.getRegionInfo().getRegionId() - l.getRegionInfo().getRegionId();' rather than check < 0 and return -1, etc? Remove a 'the' from "...It provides the the functions used to by..." Should RegionPlanComparator be public? Could it be package private? It needs to be public so the balancer package has access? Shouldn't setMasterServices instead be passed to the constructor? Otherwise, could have an instance of a balancer that was not viable, not until after setMasterServices was called (ditto w/ setClusterStatus)? So the method roundRobinAssignment is in the base load balancer. Its expected by all load balancer implementations? Its behavior will be same for all? Fix 'randmo server.' Does ClusterLoadState need to be public? Does it need audience annotation? Why a RegionInfoComparator class and then also a comparator in RegionInfo class? Is the latter a mistaken inclusion? Comment seems wrong '+ // The cache for where load balancers are located.' This is an expensive call? computeHDFSBlocksDistribution? Goes to NN? We do it for every region in cluster every time we balance? We doing to DOS the NN? The call to getTableDescriptor should be getting a descriptor from cache of table descriptors maintained by the master, right? Why ServerAndLoad implement Serializable? You planning on java serializing it? Does StochasticLoadBalancer have to be a public class? This stuff looks great. Nice tests. > 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-11.patch, HBASE-5959-2.patch, HBASE-5959-3.patch, > HBASE-5959-6.patch, HBASE-5959-7.patch, HBASE-5959-8.patch, > HBASE-5959-9.patch, HBASE-5959.D3189.1.patch, HBASE-5959.D3189.2.patch, > HBASE-5959.D3189.3.patch, HBASE-5959.D3189.4.patch, HBASE-5959.D3189.5.patch, > HBASE-5959.D3189.6.patch, HBASE-5959.D3189.7.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