hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: HBASE-2694 Move RS to Master region open/close messaging into ZooKeeper
Date Sat, 12 Jun 2010 01:01:36 GMT


> On 2010-06-11 17:35:09, Todd Lipcon wrote:
> > 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.

Thanks for review Todd.

In ZK refactor, we will do in-depth look at where we should be throwing RTE vs logging/swallowing
vs throwing.

When we do reimplementation of the open/close and all that, will look at the one you pointed
out.

Applied the following changes per your comments:

- Switched to foreach in event handler
- Switch to NONE.value
- Added TODOs for using guava and reviewing IOE vs LOG
- Added license headers to ZKUnassignedWatcher, TestZKBasedReopenRegion, TestZKBasedCloseRegion
- Added logging to noop closing method

Will put final patch up onto JIRA and commit.  Thanks!


> On 2010-06-11 17:35:09, Todd Lipcon wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java, line 1000
> > <http://review.hbase.org/r/171/diff/1/?file=1267#file1267line1000>
> >
> >     are these IOEs here and above really recoverable in any way? How could they
happen?

They wouldn't happen unless the Writable implementations were broken.  Not sure what we should
be doing here besides logging?  Better to log and ignore or throw RTE to bring attention to
something being very broken?

Adding TODO to review what we should do here


- Jonathan


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


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