hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-457) Factor Master into Master, RegionManager, and ServerManager
Date Sat, 23 Feb 2008 22:58:19 GMT

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

stack commented on HBASE-457:
-----------------------------

In the below in metascanner...

{code}
+  /** Work for the meta scanner is queued up here */
+  private volatile BlockingQueue<MetaRegion> metaRegionsToScan =
+    new LinkedBlockingQueue<MetaRegion>();
{code}

...change the volatile to final.  I think your hope was that the volatile would apply to the
content of metaRegionsToScan.  Rather it applies to the metaRegionsToScan reference... and
that ain't going to change once its been allocated (See http://java.sun.com/docs/books/jls/third_edition/html/classes.html#8.3.1.4).

You should change the few like the below in ServerManager also:

{code}
+  /** The map of known server names to server info */
+  protected volatile Map<String, HServerInfo> serversToServerInfo =
+    new ConcurrentHashMap<String, HServerInfo>();
{code}

Do data members in ServerManager have to be protected?  Can they not be private or are they
need to be protected for tests?  E.g:

{code}
+  protected HMaster master;
+  
+  protected final Leases serverLeases;
{code}

If need to be protected for tests then lets make an issue to fix this using something like
this, http://www.onjava.com/pub/a/onjava/2003/11/12/reflection.html, if its not too much of
a pain.

Does only the master have a regionManager?  If so, why not master.getRegionManager and make
the method package-private?  Then it would not have to be passed to the MetaScanner constructor?
Not important -- passing on construction saves an indirection every time... just a thought...
would help w/ the below from ProcessServerShutdown:

{code}
-          master.onlineMetaRegions.remove(info.getStartKey());
+          master.regionManager.offlineMetaRegion(info.getStartKey());
{code}

I like the cleanup done in the below:

{code}
-    ArrayList<MetaRegion> regions = new ArrayList<MetaRegion>();
-    synchronized (master.onlineMetaRegions) {
-      regions.addAll(master.onlineMetaRegions.values());
-    }
+    List<MetaRegion> regions = regionManager.getListOfOnlineMetaRegions();
{code}

(Not important), the below is a TODO?

{code}
+  /** compute the average load across all region servers */
+  public int averageLoad() {
+    return 0;
+  }
{code}

I like the new setUnassigned and noLongerPending, getRootRegionLocation, etc., methods

Fix the volatiles in RegionManager... make them finals.  You can do this now you declare and
define all in the one statement.

I ran the patch locally and it passed. Suggest that you address what you think important in
the above -- at least the volatiles I'd say -- and if tests pass locally for you, just apply
v5.

> Factor Master into Master, RegionManager, and ServerManager
> -----------------------------------------------------------
>
>                 Key: HBASE-457
>                 URL: https://issues.apache.org/jira/browse/HBASE-457
>             Project: Hadoop HBase
>          Issue Type: Sub-task
>          Components: master
>            Reporter: Bryan Duxbury
>            Assignee: Bryan Duxbury
>            Priority: Minor
>         Attachments: 457-v2.patch, 457-v3.patch, 457-v4.patch, 457.patch
>
>
> Even with TableOperation and descendants factored out of HMaster, it's still a huge class.
Every bit of master state basically lives in one class, so it's very challenging to understand
the logical groupings of everything.
> To make this a little more manageable, let's make two new abstractions, ServerManager
and RegionManager.
> ServerManager keeps track of servers - leases, message processing, load and load average,
etc. 
> RegionManager keeps track of the root location, meta table online state, assigning regions
to servers, and so on. 
> HMaster then keeps around one of each of these classes to track state and do it's master
stuff. The HMaster class itself does not change it's interface to external consumers. It also
retains the main processing loop, HBase closed state, start and stop, etc.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message