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

Ship it!


Looks good, Benoit.  Only issue is in LocalHBaseCluster where default behavior 
looks different now.  I can fix that on commit, or just change that before 
putting the patch up on JIRA?


trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.hbase.org/r/132/#comment785>

    This seems to change behavior.  Looks like it would previously default to 
local if not set, now will default to non local?



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

    From this point forward in the diff, it looks like a majority of the 
changes are whitespace.  Not saying you need to throw it out but it's making 
this long patch even longer :)



trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
<http://review.hbase.org/r/132/#comment787>

    I guess there were a few tests that implemented HConstants but didn't 
actually use anything in there?  Maybe was before HBaseTestCase


- Jonathan


On 2010-06-07 13:26:26, Benoit Sigoure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/132/
> -----------------------------------------------------------
> 
> (Updated 2010-06-07 13:26:26)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2618 Don't inherit from HConstants.
> 
> Bonus: minor aesthetic / coding style clean ups and minor code changes.
> 
> 
> This addresses bug HBASE-2618.
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/HMerge.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/BaseScanner.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java 
> 951935 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/master/RegionServerOperation.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/TableOperation.java 
> 951935 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java
>  951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKServerTool.java 
> 951935 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 
> 951935 
>   trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 951935 
>   trunk/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java 
> 951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetDeleteTracker.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
>  951935 
>   
> trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestWildcardColumnTracker.java
>  951935 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java 
> 951935 
> 
> Diff: http://review.hbase.org/r/132/diff
> 
> 
> Testing
> -------
> 
> Code compiles.
> Tests are acting up on my machine right now (many of them fail with a weird 
> message [1] and Todd says he's been seeing similar failures for some time 
> already, so I guess I'll try to run them again next week when the New Moon 
> arrives).
> 
> 
> [1] A number of tests fail with:
> org.apache.hadoop.hbase.client.NoServerForRegionException: Timed out trying 
> to locate root region because: Failed setting up proxy to /192.168.0.7:63773 
> after attempts=1
>         at 
> org.apache.hadoop.hbase.client.HConnectionManager$TableServers.locateRootRegion(HConnectionManager.java:1031)
> Where, of course, 192.168.0.7 is my IP address.
> Some of the tests that are acting up:
> org.apache.hadoop.hbase.TestZooKeeper, 
> org.apache.hadoop.hbase.regionserver.wal.TestLogRolling, 
> org.apache.hadoop.hbase.rest.TestScannersWithFilters, 
> org.apache.hadoop.hbase.master.TestMasterWrongRS, 
> org.apache.hadoop.hbase.thrift.TestThriftServer, 
> org.apache.hadoop.hbase.master.TestMasterTransitions, 
> org.apache.hadoop.hbase.rest.TestStatusResource, 
> org.apache.hadoop.hbase.client.TestFromClientSide, 
> org.apache.hadoop.hbase.TestMultiParallelPut, 
> org.apache.hadoop.hbase.master.TestRegionManager, 
> org.apache.hadoop.hbase.mapreduce.TestTimeRangeMapRed
> 
> 
> Thanks,
> 
> Benoit
> 
>

Reply via email to