----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1176/#review1852 -----------------------------------------------------------
Patch looks great. Needs the test filled out before +1 and do all unit tests pass? It changes a bunch of critical code so wouldn't be surprised if unexpected side effects. trunk/pom.xml <http://review.cloudera.org/r/1176/#comment6013> You didn't mean this, right? We can't have direct cloudera dependency. trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java <http://review.cloudera.org/r/1176/#comment6014> No one depends on these? trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java <http://review.cloudera.org/r/1176/#comment6015> review board is showing a bunch of white space on these lines? Tabs? trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java <http://review.cloudera.org/r/1176/#comment6016> You changing public API here? I suppose we are but only in HCM which is rarely used by other than internals... disregard this comment since I see you convert it in HTable below to an IOE. trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java <http://review.cloudera.org/r/1176/#comment6018> Hmmm... would make life easier if we just let out the IE. trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java <http://review.cloudera.org/r/1176/#comment6020> Long line. trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java <http://review.cloudera.org/r/1176/#comment6023> Yeah, cleaner if IE is let out (I suppose). trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java <http://review.cloudera.org/r/1176/#comment6024> This is kinda ugly. For sure exceptions have been purged here? trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java <http://review.cloudera.org/r/1176/#comment6025> Adding an IE here is ok? Is this a new method in 0.90? trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java <http://review.cloudera.org/r/1176/#comment6026> Oh, ok... then IE is fine. trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java <http://review.cloudera.org/r/1176/#comment6027> Could there be exception here or its always Result? If always Result why change type to Object? trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java <http://review.cloudera.org/r/1176/#comment6028> Missing class comment on what this doohickey does trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java <http://review.cloudera.org/r/1176/#comment6029> Hey man, great test! Can you add something in here? Smile. - stack On 2010-11-05 01:43:22, Ryan Rawson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1176/ > ----------------------------------------------------------- > > (Updated 2010-11-05 01:43:22) > > > Review request for hbase. > > > Summary > ------- > > This is a change to batch() that substantially increases the error handling. > Included is a new Exception type with a lot more details of what failed and > where. Users will be able to also query the thrown exception to see if there > is a possible cluster problem. They will be able to log the server addresses > that had issues. > > The TestMultiParallel test case passes, I need to run the rest. > > > This addresses bug HBASE-2898. > http://issues.apache.org/jira/browse/HBASE-2898 > > > Diffs > ----- > > trunk/pom.xml 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java > 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java > 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java > 1031470 > > trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java > PRE-CREATION > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > 1031470 > > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerStoppedException.java > 1031470 > trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java > 1031470 > trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java > PRE-CREATION > trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java > 1031470 > > Diff: http://review.cloudera.org/r/1176/diff > > > Testing > ------- > > > Thanks, > > Ryan > >
