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

Ted Yu commented on HBASE-6721:
-------------------------------

{code}
+  public LoadBalancer getBalancer() {
+    return balancer;
+  }
{code}
The above method can be package private.
{code}
+public class GroupAdminEndpoint extends BaseEndpointCoprocessor implements 
GroupAdminProtocol, EventHandler.EventHandlerListener {
+ private static final Log LOG = LogFactory.getLog(GroupAdminClient.class);
{code}
Please add javadoc for the class. The line is beyond 100 characters.
Log has wrong class.
{code}
+  private ConcurrentMap<String,String> serversInTransition =
{code}
What does the value in serversInTransition map represent ?
{code}
+   List<HRegionInfo> regions = new ArrayList<HRegionInfo>();
+   if (groupName == null) {
+      throw new NullPointerException("groupName can't be null");
{code}
nit: move ArrayList creation after the if statement.
{code}
+  public Collection<String> listTablesOfGroup(String groupName) throws 
IOException {
{code}
The return type is a collection, more generic than List that 
listOnlineRegionsOfGroup() returns. I guess there might be a reason.
{code}
+      HTableDescriptor[] tables = 
master.getTableDescriptors().getAll().values().toArray(new HTableDescriptor[0]);
{code}
nit: line too long.
{code}
+  public GroupInfo getGroup(String groupName) throws IOException {
{code}
Suggest renaming the method getGroupInfo(). getGroup() is kind of vague.

More reviews to follow.
                
> RegionServer Group based Assignment
> -----------------------------------
>
>                 Key: HBASE-6721
>                 URL: https://issues.apache.org/jira/browse/HBASE-6721
>             Project: HBase
>          Issue Type: New Feature
>            Reporter: Francis Liu
>            Assignee: Vandana Ayyalasomayajula
>             Fix For: 0.96.0
>
>         Attachments: HBASE-6721_94.patch, HBASE-6721_94.patch, 
> HBASE-6721-DesigDoc.pdf
>
>
> In multi-tenant deployments of HBase, it is likely that a RegionServer will 
> be serving out regions from a number of different tables owned by various 
> client applications. Being able to group a subset of running RegionServers 
> and assign specific tables to it, provides a client application a level of 
> isolation and resource allocation.
> The proposal essentially is to have an AssignmentManager which is aware of 
> RegionServer groups and assigns tables to region servers based on groupings. 
> Load balancing will occur on a per group basis as well. 
> This is essentially a simplification of the approach taken in HBASE-4120. See 
> attached document.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to