[
https://issues.apache.org/jira/browse/HBASE-5083?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13692245#comment-13692245
]
Jesse Yates edited comment on HBASE-5083 at 6/24/13 6:51 PM:
-------------------------------------------------------------
Looking at the trunk patch, a couple of nits:
in
hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/BackupMasterStatusTmpl.jamon
{quote}
+Copyright 2011 The Apache Software Foundation
{quote}
isn't needed.
{quote}
+ </tr>
+ <%java>
+ Arrays.sort(serverNames);
+ for (ServerName serverName: serverNames) {
+ </%java>
{quote}
Spacing looks a little bit off.
In HMaster,
{quote}
+ if(master.isActiveMaster()){
+ metaLocation = getMetaLocationOrNull(master);
+ //ServerName metaLocation =
master.getCatalogTracker().getMetaLocation();
+ servers = master.getServerManager().getOnlineServersList();
+ deadServers =
master.getServerManager().getDeadServers().copyServerNames();
+ }
{quote}
Spacing is off (everything else looks to be 2 spaces, not 4 (or is that a tab?
can't tell just reading the text diff)).
{quote}
+ return (master.getCatalogTracker() == null) ? null :
master.getCatalogTracker().getMetaLocation();
{quote}
Wish there wasn't a need for the null here and instead a special ServerName
that we could use when its null (increases potential for NullPointerExceptions,
makes code a little more brittle, requires more null checks other places,
etc.), but just complaining - this is fine and fits in with everything else.
otherwise, it looks fine.
+1, if you wouldn't mind fixing the nits on commit.
was (Author: jesse_yates):
nits:
in
hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/BackupMasterStatusTmpl.jamon
{quote}
+Copyright 2011 The Apache Software Foundation
{quote}
isn't needed.
{quote}
+ </tr>
+ <%java>
+ Arrays.sort(serverNames);
+ for (ServerName serverName: serverNames) {
+ </%java>
{quote}
Spacing looks a little bit off.
In HMaster,
{quote}
+ if(master.isActiveMaster()){
+ metaLocation = getMetaLocationOrNull(master);
+ //ServerName metaLocation =
master.getCatalogTracker().getMetaLocation();
+ servers = master.getServerManager().getOnlineServersList();
+ deadServers =
master.getServerManager().getDeadServers().copyServerNames();
+ }
{quote}
Spacing is off (everything else looks to be 2 spaces, not 4 (or is that a tab?
can't tell just reading the text diff)).
{quote}
+ return (master.getCatalogTracker() == null) ? null :
master.getCatalogTracker().getMetaLocation();
{quote}
Wish there wasn't a need for the null here and instead a special ServerName
that we could use when its null (increases potential for NullPointerExceptions,
makes code a little more brittle, requires more null checks other places,
etc.), but just complaining - this is fine and fits in with everything else.
otherwise, it looks fine.
+1, if you wouldn't mind fixing the nits on commit.
> Backup HMaster should have http infoport open with link to the active master
> ----------------------------------------------------------------------------
>
> Key: HBASE-5083
> URL: https://issues.apache.org/jira/browse/HBASE-5083
> Project: HBase
> Issue Type: Improvement
> Components: master
> Affects Versions: 0.92.0
> Reporter: Jonathan Hsieh
> Assignee: Cody Marcel
> Fix For: 0.94.9
>
> Attachments: backup_master.png, HBASE-5083.patch, HBASE-5083.patch,
> HBASE-5083.patch, HBASE-5083.patch, HBASE-5083.patch, HBASE-5083_trunk.patch,
> HBASE-5083_trunk.patch, HBASE-5083_trunk.patch, master.png,
> Trunk_Backup_Master.png, Trunk_Master.png
>
>
> Without ssh'ing and jps/ps'ing, it is difficult to see if a backup hmaster is
> up. It seems like it would be good for a backup hmaster to have a basic web
> page up on the info port so that users could see that it is up. Also it
> should probably either provide a link to the active master or automatically
> forward to the active master.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
