Review Board 1.7.22


HBASE-1502 Remove need for heartbeats in HBase

Review Request #647 - Created April 22, 2011 and updated

Michael Stack
trunk
hbase-1502
Reviewers
hbase
hbase-git
This patch does not completely remove heartbeats.  It unburdens the heartbeat of control messages; now heartbeat is used to
send the master load only (At most recent hackathon we had rough agreement that we'd keep heartbeat to carry load)... if we miss some, no biggie.   

RPC version changed on HMasterRegionInfo since the regionServerStartup and regionServerReport arguments have changed.
We pass a String now instead of HServerAddress so this should help with our DNS issues where the two sides disagree.

Removed HMsg.

HServerAddress as been sort_of_deprecated.  Its in our API so can't remove it easily (its embedded inside HRegionLocation).
Otherwise, we don't use it internally anymore.

HServerInfo is deprecated.  Server meta data is now available in new class ServerName and load lives apart from HSI now.

Fixed up regionserver and master startup so they now look the same.

New tests

Cruft cleanup.
Most tests pass.  I have yet to run it on a cluster.  Doing that now.
Review request changed
Updated (April 26, 2011, 11:50 p.m.)
All tests pass now.  I'd like to get this patch in soon.  I'm currently spending a good bit of my time trying to keep this patch up with current TRUNK.  I'd rather commit and then address issues after.

This version of the patch does one  make significant change though in that it deprecates prewarmRegionCache.  IMO this is a burdensome feature that is little used; i'd like to have it die off.
Ship it!
Posted (April 27, 2011, 12:17 a.m.)
woohoo!  glad HMsg is dead!
Can just use this.liveServers.values() for here and below?
  1. value is HServerLoad (Not a List of HRegionInfo).  We want count of regions.
    
    I did remove the dup iteration having getAverageLoad instead call getRegionsCount.
Where is this actually used now?  Should point it out here so it's clear and so that when it goes away we know we can get rid of this.
  1. Will do.  Had a change of heart last night; previous was going to just 'sort-of' deprecate HSA but on way  home, after thinking on it, HSA is just never a good idea so will outright deprecate it.
i see webuiport below, does this TODO still apply?
  1. It does.
    
    HSI is deprecated and after this patch goes in, there is no means of the RS telling Master of its webui port -- in master we'll rely on configs.  In rare case where webui port changes -- it can if occupied -- then master will be off.
    
    Will address elsewhere.  Will write json to the RS ephemeral node with 'metadata' about the RS that will include webui port but likely other stuff that could be factored doing loading calculations, etc. (it used to hold a HSA but thats been bumped in this patch).  Will file an issue after this patch goes in.
why String and not ServerName?  because master has no startcode?  (i see use of ServerName for master above tho)
  1. I changed it (This method is not used anywhere seemingly).
awesome that this is tucked away in here now
this is because HSA actually makes a connection or does the lookup?
  1. This I'll purge.  I'll use the getConnection(ISA) that this patch adds instead so no reference to HSA in HBaseAdmin (except deprecated uses)
Posted (April 27, 2011, 5:41 p.m.)
Thanks for review jg
This I've been changing removing HSA.  Instead I'm passing an ISA (this patch adds a getConnection(isa) method).