----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1262/#review2013 -----------------------------------------------------------
Ship it! A little confused by the discrepancy between String host / int port and the Address. But does seem fine given we don't actually access the string/int values and always use the address object. Do we need some tests on this stuff? Seems like we always have issues here but tests don't catch anything. Looks better than what we have though so I'm +1 regardless. - Jonathan On 2010-11-30 11:45:53, Jean-Daniel Cryans wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1262/ > ----------------------------------------------------------- > > (Updated 2010-11-30 11:45:53) > > > Review request for hbase. > > > Summary > ------- > > Changes: > - In HMaster, instead of passing an IP as String we now pass the HSA object > completely. > - In HRegionServer, I cleared a bunch of crufty comments and handle the HSA > passed by the master. > - In HServerInfo, I saw that the hostname wasn't reset when setting the HSA. > Fixed. > - In HServerAddress, I fixed a few places that wasn't explicitly using > hostnames and changed the serialization to pass a hostname instead of an IP > address. > > > This addresses bug HBASE-3286. > http://issues.apache.org/jira/browse/HBASE-3286 > > > Diffs > ----- > > /trunk/src/main/java/org/apache/hadoop/hbase/HServerAddress.java 1040669 > /trunk/src/main/java/org/apache/hadoop/hbase/HServerInfo.java 1040669 > /trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1040669 > > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > 1040669 > > Diff: http://review.cloudera.org/r/1262/diff > > > Testing > ------- > > Works on my MBP (I was seeing the same issue but since there's only 1 RS it > didn't have any bad effect) and my 10 machines Ubuntu cluster. > > > Thanks, > > Jean-Daniel > >
