Review Board 1.7.22


hbase-5328 Small changes to Master to make it more testable

Review Request #4436 - Created March 21, 2012 and updated

Michael Stack
hbase-5328
Reviewers
hbase
hbase-git
M src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
  Make this class public so its waitForRoot(long) can be used by HMaster.
  Remove the stalling waitForRoot no arg.
M src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
  Javadoc.  Add check if stopped flag cycling waiting on assignment.
M src/main/java/org/apache/hadoop/hbase/master/HMaster.java
  Moved check if schema change flag out to a method rather than
  have it on tail of constructor.
  Moved other initialization stuff like get of assignment manager
  and server manager out into methods so could be intercepted by
  tests and mocking.
  Change how we wait on root so we sleep 100ms at a time and always
  check stopped flag rather than block for ever.
  Added more checking if stopped flag.
  Added flag for when rpc server is up, mostly for tests.
M src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java
  Unused import.
M src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java
  Comment.
M src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
  Remove unused code.
M src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
  Change how we wait on root.  DOn't use removed method.
A src/test/java/org/apache/hadoop/hbase/master/MockRegionS

 
Review request changed
Updated (March 21, 2012, 11:18 p.m.)
Address Ted comments.
Posted (March 22, 2012, 12:25 a.m.)

   

  
  1. Let me add addendum to address two of your feedbacks below Jon.
nit: comment is wrong for change.
  1. I fixed this in an addendum.
was: remove all instances of "TODO Auto-generated method stub"?  

Replace with a comment to say this should be overridden (or throw some sort of RuntimeException?)
  1. Its a mock.  I want default, basic behaviors. 
  2. I'm not going to push too hard here, but leaving the auto gen stub generally tells me "incomplete code" instead of telling "basic behavior" or "not default behavior".
  3. I looked at this this morning again since it reviewers balk.   I tried to come up w/ text to put in place of what was auto-generated but anything I conjured seemed inauthentic compared to what the machine generated so I am going to just leave it (It shouldn't be overridden, at least not currently, not until we need the mock to do more, and neither do I want to throw runtime exceptions, etc). 
make these joins have a timeout (prevent hanging tests?)
  1. Done in an addendum.
Ship it!
Posted (March 22, 2012, 3:30 p.m.)
Sorry, meant to have this said this after the first review.