----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1271/#review2037 -----------------------------------------------------------
Ship it! Looks good Ted. Below are a few pointers mostly on formatting and then a few questions. Thanks for making the patch. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/1271/#comment6430> Do you need to pollute HMaster with this AssignmentManager inner class? trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/1271/#comment6431> FYI, don't make these kinda formatting changes in a patch... its distracting and the change you are making is against the convention used in the rest of this file. Just FYI. No biggie. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/1271/#comment6432> Yeah, maybe these lines belong inside a method that is inside AssignmentManager? What you think Ted? trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java <http://review.cloudera.org/r/1271/#comment6433> What changed on this line? White space? trunk/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java <http://review.cloudera.org/r/1271/#comment6434> Convention is two spaces for tab in hbase and hadoop. This seems like something else? trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java <http://review.cloudera.org/r/1271/#comment6435> FYI, tab is two spaces... we indent in multiples of two spaces. trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java <http://review.cloudera.org/r/1271/#comment6437> Good. Nice test. - stack On 2010-12-06 10:42:26, Ted Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1271/ > ----------------------------------------------------------- > > (Updated 2010-12-06 10:42:26) > > > Review request for hbase, stack and Jonathan Gray. > > > Summary > ------- > > Adopted round-robin assignment as default for regions specified when table is > created. > > > This addresses bug HBASE-3305. > http://issues.apache.org/jira/browse/HBASE-3305 > > > Diffs > ----- > > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1042725 > trunk/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java > 1042725 > trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java 1042725 > > Diff: http://review.cloudera.org/r/1271/diff > > > Testing > ------- > > Put unit tests for this change inside TestAdmin.testCreateTableWithRegions() > They passed. > > > Thanks, > > Ted > >
