You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Jean-Daniel Cryans <jd...@apache.org> on 2010/11/30 20:45:53 UTC
Review Request: Master passes IP and not hostname back to region server
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1262/
-----------------------------------------------------------
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
Re: Review Request: Master passes IP and not hostname back to region server
Posted by Jean-Daniel Cryans <jd...@apache.org>.
> On 2010-11-30 12:27:06, Jonathan Gray wrote:
> > 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.
Regarding tests, I'm not sure what they would catch...
> On 2010-11-30 12:27:06, Jonathan Gray wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/HServerAddress.java, line 65
> > <http://review.cloudera.org/r/1262/diff/1/?file=17919#file17919line65>
> >
> > Why does stringValue not necessarily equal the host:port we store in those Strings? Shouldn't they be the same?
I'm trying to keep it more consistent with the rest of the code, else when looking at the code you ask yourself the question you just asked me :)
> On 2010-11-30 12:27:06, Jonathan Gray wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/HServerAddress.java, line 177
> > <http://review.cloudera.org/r/1262/diff/1/?file=17919#file17919line177>
> >
> > But on serialization, we use the address hostname not the thing we actually store in hostname/port variables, so after serialized it's different?
> >
> > Shouldn't we set the hostname/port variables on construction according to address.getAddress/getPort rather than the passed values, if the address values are what we want to use?
I'm... not following you. You're saying that we shouldn't store the InetSocketAddress?
> On 2010-11-30 12:27:06, Jonathan Gray wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/HServerInfo.java, line 116
> > <http://review.cloudera.org/r/1262/diff/1/?file=17920#file17920line116>
> >
> > I guess we never actually use the String host / int port? Why do we store them in HServerAddress then?
Here I'm just making sure that after updating the address we also update the hostname, since it could have changed.
- Jean-Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1262/#review2012
-----------------------------------------------------------
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
>
>
Re: Review Request: Master passes IP and not hostname back to region server
Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1262/#review2012
-----------------------------------------------------------
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.
/trunk/src/main/java/org/apache/hadoop/hbase/HServerAddress.java
<http://review.cloudera.org/r/1262/#comment6351>
Why does stringValue not necessarily equal the host:port we store in those Strings? Shouldn't they be the same?
/trunk/src/main/java/org/apache/hadoop/hbase/HServerAddress.java
<http://review.cloudera.org/r/1262/#comment6352>
But on serialization, we use the address hostname not the thing we actually store in hostname/port variables, so after serialized it's different?
Shouldn't we set the hostname/port variables on construction according to address.getAddress/getPort rather than the passed values, if the address values are what we want to use?
/trunk/src/main/java/org/apache/hadoop/hbase/HServerInfo.java
<http://review.cloudera.org/r/1262/#comment6353>
I guess we never actually use the String host / int port? Why do we store them in HServerAddress then?
/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.cloudera.org/r/1262/#comment6354>
whitespace
- 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
>
>
Re: Review Request: Master passes IP and not hostname back to region server
Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
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
>
>