hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: HBASE-1748 ClusterStatus needs to print out who has master role
Date Mon, 07 Jun 2010 21:48:27 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/162/#review145
-----------------------------------------------------------


Looks good.  Some questions in the below.


src/main/java/org/apache/hadoop/hbase/ClusterStatus.java
<http://review.hbase.org/r/162/#comment749>

    Should it be hosname rather than ip?



src/main/java/org/apache/hadoop/hbase/ClusterStatus.java
<http://review.hbase.org/r/162/#comment750>

    should it include a port too?



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.hbase.org/r/162/#comment751>

    Does this have to be done on each access to getClusterStatus?  Can we not do this once
soon as this master assumes the master role?
    
    I see now where you are getting the master ip from. Do we have master ip up in zk?  If
so, that is what is confining you I'd imagine.
    
    It shoudl be hostname and port up in ZK?  If so, we can do that in a separate issue but
go ahead and commit this as is for now.



src/main/java/org/apache/hadoop/hbase/util/InfoServer.java
<http://review.hbase.org/r/162/#comment752>

    This change is incorrect, no?  webapps are currently at hbase-webapps (though distributed
hbase ui is broke... )
    



src/main/java/org/apache/hadoop/hbase/util/InfoServer.java
<http://review.hbase.org/r/162/#comment753>

    This is great.



src/main/java/org/apache/hadoop/hbase/util/InfoServer.java
<http://review.hbase.org/r/162/#comment754>

    Oh, you are changing this back intentionally?  Whats up here?



src/main/java/org/apache/hadoop/hbase/util/MasterLocatorFilter.java
<http://review.hbase.org/r/162/#comment755>

    Should this filter go into the master webapp?


- stack


On 2010-06-07 14:30:25, Lars George wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/162/
> -----------------------------------------------------------
> 
> (Updated 2010-06-07 14:30:25)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-1748 ClusterStatus needs to print out who has master role
> 
> Please have a look, I was not able to test (yet) on a single machine. Will try on cluster
later but wanted to get a quick feedback. 
> 
> Also upped VERSION in ClusterStatus, is that OK or not needed?
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ClusterStatus.java d102643 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java 5946eee 
>   src/main/java/org/apache/hadoop/hbase/util/InfoServer.java 7a3d4f9 
>   src/main/java/org/apache/hadoop/hbase/util/MasterLocatorFilter.java PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/162/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lars
> 
>


Mime
View raw message