hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars George" <larsgeo...@apache.org>
Subject Re: Review Request: HBASE-1748 ClusterStatus needs to print out who has master role
Date Mon, 07 Jun 2010 22:05:45 GMT


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 40
> > <http://review.hbase.org/r/162/diff/1/?file=1056#file1056line40>
> >
> >     Should it be hosname rather than ip?

The reason is that when the ZooKeeper node is created an IP address is stored, not the hostname.
After that it is all IP based by the looks. I am reading from the ZK node using ZKW and it
sticks with the IP representation. Should we change all that?


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/ClusterStatus.java, line 175
> > <http://review.hbase.org/r/162/diff/1/?file=1056#file1056line175>
> >
> >     should it include a port too?

I was thinking about that too, but with the current setup the master must have the same port
on all machines, even on the standby masters, right? That is why I omitted it but I am open
to suggestions.


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 1035
> > <http://review.hbase.org/r/162/diff/1/?file=1057#file1057line1035>
> >
> >     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.

Yeah, see my noted above. We would need to clean this up in a separate issue I'd say. We could
also add all standby masters to list themselves up in ZK so that we can report on that too,
i.e. active master and standby ones.


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/util/InfoServer.java, line 39
> > <http://review.hbase.org/r/162/diff/1/?file=1058#file1058line39>
> >
> >     This change is incorrect, no?  webapps are currently at hbase-webapps (though
distributed hbase ui is broke... )
> >

This must have slipped in during my rebasing and copying around. Not correct.


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/util/InfoServer.java, line 123
> > <http://review.hbase.org/r/162/diff/1/?file=1058#file1058line123>
> >
> >     Oh, you are changing this back intentionally?  Whats up here?

See above, regression and I will clean this up.


> On 2010-06-07 14:48:27, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/util/MasterLocatorFilter.java, line 41
> > <http://review.hbase.org/r/162/diff/1/?file=1059#file1059line41>
> >
> >     Should this filter go into the master webapp?

I put it next to InfoServer where it is used. What are you suggesting? I think I am misreading
your comment, could you elaborate?


- Lars


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


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