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:
-------------------------------

{code}
+public class AssignmentDomain {
{code}
Please add class javadoc and audience annotation.
{code}
+   * 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 {
{code}
Please add javadoc explaining what skipRackSet represents.
If I read the code correctly, not just the current rack could be skipped.
{code}
+  public void addServer(ServerName server) {
+    // For a new server
+    String rackName = this.rackManager.getRack(server);
{code}
Is it possible that null gets returned from getRack() ?
{code}
+ * Copyright 2012 The Apache Software Foundation
{code}
Year appears in copyright in a few files. It is not needed.

For AssignmentPlan:
{code}
+ * AssignmentPlan is a writable object for the region assignment plan.
{code}
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:
{code}
+  public synchronized void updateAssignmentPlan(HRegionInfo region,
+      List<ServerName> servers) {
{code}
{code}
+    if (updateTS == null)
+      return Long.MIN_VALUE;
+    else
{code}
nit: 'else' is not needed. Introduce curly braces if you keep the 'return' on separate line.
{code}
+  public synchronized Map<HRegionInfo, List<ServerName>> getAssignmentMap() {
+    return this.assignmentMap;
{code}
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

Mime
View raw message