FAQ
-----------------------------------------------------------
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

Search Discussions

  • Jonathan Gray at Nov 30, 2010 at 8:27 pm
    -----------------------------------------------------------
    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
  • Jean-Daniel Cryans at Dec 1, 2010 at 4:09 pm

    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
  • Jonathan Gray at Nov 30, 2010 at 8:27 pm
    -----------------------------------------------------------
    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdev @
categorieshbase, hadoop
postedNov 30, '10 at 8:08p
activeDec 1, '10 at 4:09p
posts4
users2
websitehbase.apache.org

People

Translate

site design / logo © 2022 Grokbase