hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ted Yu (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-7932) Do the necessary plumbing for the region locations in META table and send the info to the RegionServers
Date Wed, 27 Feb 2013 00:14:12 GMT

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

Ted Yu commented on HBASE-7932:

+public class AssignmentDomain {
Please add class javadoc and audience annotation.
+   * Get a random rack except for the current rack
+   * @param skipRackSet
+   * @return the random rack except for any Rack from the skipRackSet
+   * @throws IOException
+   */
+  public String getOneRandomRack(Set<String> skipRackSet) throws IOException {
Please add javadoc explaining what skipRackSet represents.
If I read the code correctly, not just the current rack could be skipped.
+  public void addServer(ServerName server) {
+    // For a new server
+    String rackName = this.rackManager.getRack(server);
Is it possible that null gets returned from getRack() ?
+ * Copyright 2012 The Apache Software Foundation
Year appears in copyright in a few files. It is not needed.

For AssignmentPlan:
+ * AssignmentPlan is a writable object for the region assignment plan.
We don't want Writable in 0.95 and beyond, right ?
Please add audience annotation.

+  /** the map between each region and its lasted favored server list update

lasted: typo

Two maps are used in AssignmentPlan. I assume they are updated in sync.
But in the following method, only assignmentMap is updated:
+  public synchronized void updateAssignmentPlan(HRegionInfo region,
+      List<ServerName> servers) {
+    if (updateTS == null)
+      return Long.MIN_VALUE;
+    else
nit: 'else' is not needed. Introduce curly braces if you keep the 'return' on separate line.
+  public synchronized Map<HRegionInfo, List<ServerName>> getAssignmentMap() {
+    return this.assignmentMap;
Do we need to clone the map before returning ?

More reviews to follow.
> Do the necessary plumbing for the region locations in META table and send the info to
the RegionServers
> -------------------------------------------------------------------------------------------------------
>                 Key: HBASE-7932
>                 URL: https://issues.apache.org/jira/browse/HBASE-7932
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Devaraj Das
>            Assignee: Devaraj Das
>            Priority: Critical
>             Fix For: 0.95.0
>         Attachments: 7932-wip-2.patch, 7932-wip-3.patch, 7932-wip-4.patch, 7932-wip.patch

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

View raw message