hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon" <t...@cloudera.com>
Subject Re: Review Request: HBASE-2694 Move RS to Master region open/close messaging into ZooKeeper
Date Sat, 12 Jun 2010 00:35:09 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/171/#review202
-----------------------------------------------------------

Ship it!


Skimmed it reasonably quickly, looks good.
Major concern is how often we catch and just log exceptions. In most cases I think these are
errors we can't recover from and it's best to throw RTE and shut down the whole server.

Several places missing license headers.


trunk/src/main/java/org/apache/hadoop/hbase/executor/HBaseEventHandler.java
<http://review.hbase.org/r/171/#comment918>

    use foreach form?



trunk/src/main/java/org/apache/hadoop/hbase/executor/HBaseEventHandler.java
<http://review.hbase.org/r/171/#comment919>

    foreach form



trunk/src/main/java/org/apache/hadoop/hbase/executor/HBaseExecutorService.java
<http://review.hbase.org/r/171/#comment920>

    == NONE.value, or if (this == NONE)?



trunk/src/main/java/org/apache/hadoop/hbase/executor/NamedThreadFactory.java
<http://review.hbase.org/r/171/#comment921>

    You can just use this one:
    
    http://guava-libraries.googlecode.com/svn/trunk/javadoc/com/google/common/util/concurrent/NamingThreadFactory.html



trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java
<http://review.hbase.org/r/171/#comment922>

    are these IOEs here and above really recoverable in any way? How could they happen?



trunk/src/main/java/org/apache/hadoop/hbase/master/ZKUnassignedWatcher.java
<http://review.hbase.org/r/171/#comment923>

    license



trunk/src/main/java/org/apache/hadoop/hbase/master/handler/MasterOpenRegionHandler.java
<http://review.hbase.org/r/171/#comment924>

    maybe add a LOG.debug here at least for now



trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/171/#comment925>

    again, are these LOG.errors really recoverable things?



trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/171/#comment926>

    recoverable?



trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java
<http://review.hbase.org/r/171/#comment927>

    can't wait til we refactor all this stuff



trunk/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedCloseRegion.java
<http://review.hbase.org/r/171/#comment928>

    license
    


- Todd


On 2010-06-11 16:24:15, Karthik Ranganathan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/171/
> -----------------------------------------------------------
> 
> (Updated 2010-06-11 16:24:15)
> 
> 
> Review request for hbase, Todd Lipcon and stack.
> 
> 
> Summary
> -------
> 
> Moved all RS -> Master communication to go through ZK so that we can enable master
failover.
> 
> 
> This addresses bug HBASE-2694.
>     http://issues.apache.org/jira/browse/HBASE-2694
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/executor/HBaseEventHandler.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/executor/HBaseExecutorService.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/executor/NamedThreadFactory.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/executor/RegionTransitionEventData.java
PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ProcessRegionClose.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ProcessRegionStatusChange.java 953804

>   trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ZKMasterAddressWatcher.java 953804

>   trunk/src/main/java/org/apache/hadoop/hbase/master/ZKUnassignedWatcher.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/master/handler/MasterCloseRegionHandler.java
PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/handler/MasterOpenRegionHandler.java
PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 953804

>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RSZookeeperUpdater.java PRE-CREATION

>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java 953804 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 953804

>   trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 953804 
>   trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java 953804 
>   trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java 953804 
>   trunk/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedCloseRegion.java PRE-CREATION

>   trunk/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedReopenRegion.java PRE-CREATION

> 
> Diff: http://review.hbase.org/r/171/diff
> 
> 
> Testing
> -------
> 
> Unit tested.
> 
> 
> Thanks,
> 
> Karthik
> 
>


Mime
View raw message