HBASE-3260: Add explicit lifecycle methods to Coprocessor interface

Review Request #1306 - Created Dec. 16, 2010 and submitted

Gary Helmling
old-hbase
trunk
HBASE-3260
hbase
apurtell, stack
This patch adds explicit start() and stop() methods for lifecycle management to the Coprocessor interface and refactors some of the Coprocessor/RegionObserver distinction, moving the region-related pre/post hooks that were previously in Coprocessor to RegionObserver.

Coprocessor is now the base interface, containing only:
 - start()
 - stop()
 - Priority enum
 - State enum

RegionObserver extends Coprocessor, and now contains the additional pre/post hooks, moved from Coprocessor:
 - pre/postOpen
 - pre/postClose
 - pre/postFlush
 - pre/postCompact
 - pre/postSplit

This will allow cleaner extension in the future, to allow addition of a MasterObserver interface, for example.

As shown above, I've also added a new Coprocessor.State enum consisting of the states:
UNINSTALLED -> INSTALLED -> STARTING -> ACTIVE -> STOPPING -> STOPPED

However, the UNINSTALLED/INSTALLED distinction is not particularly useful at the moment.  I'd appreciate other feedback on what's necessary here.  The current handling could make do with:
UNINSTALLED -> STARTING -> ACTIVE -> STOPPING -> UNINSTALLED (4 total states)

However, the UNINSTALLED/INSTALLED distinction may be useful if we want to add class level initialization in the future...
Added tests for start() and stop() method invocation in org.apache.hadoop.hbase.coprocessor.TestCoprocessorInterface

The existing TestCoprocessorEndpoint, TestCoprocessorInterface, TestRegionObserverInterface, TestRegionObserverStacking tests continue to work.  I'm not seeing any new failures in the rest of the tests, but TestReplication is timing out for me, preventing all tests from executing.
  1. 
      
  2. What are these arguments about?
    1. Those are:
      - String protocol
      - long clientVersion
      
      from org.apache.hadoop.ipc.VersionedProtocol.
      
      Will fix these up.
  3. Since you are committing a change set in this area, Ryan suggested no need for AtomicBoolean here, could just be plain volatile boolean. I think that's right.
    1. Ok will change this to a volatile boolean and repost.
  4. 
      
Loading...