hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: HBASE-3256: Coprocessors: Coprocessor host and observer for HMaster
Date Tue, 21 Dec 2010 06:39:34 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1321/#review2132
-----------------------------------------------------------

Ship it!


looks good.  clean and nicely commented, well done.

- Jonathan


On 2010-12-20 22:31:38, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1321/
> -----------------------------------------------------------
> 
> (Updated 2010-12-20 22:31:38)
> 
> 
> Review request for hbase, stack, Andrew Purtell, and Jonathan Gray.
> 
> 
> Summary
> -------
> 
> This patch adds a new MasterObserver interface with pre/post hooks provided for operations
defined in org.apache.hadoop.hbase.ipc.HMasterInterface.
> 
> In order to accommodate the new MasterObserver interface, I've also refactored out common
coprocessor base code, with subclasses providing for region-specific and master-specific behavior.
> 
> The new code structure is (excuse my poor ascii art):
> 
> CoprocessorEnvironment - base interface for common facilities provided to CP implementations
>     | 
>     |- RegionCoprocessorEnvironment - adds access to current HRegion and RegionServerServices
(for RegionObservers)
>     |
>     |- MasterCoprocessorEnvironment - adds access to MasterServerServices (for MasterObservers)
> 
> CoprocessorHost - abstract base providing core CP loading and invocation code and the
base CoprocessorEnvironment implementation
>     |
>     |- RegionCoprocessorHost - provides hooks for invoking RegionObserver pre/post methods
and RegionCoprocessorEnvironment implementation
>     |
>     |- MasterCoprocessorHost - provides hooks for invoking MasterObserver pre/post methods
and MasterCoprocessorEnvironment implementation
> 
> Also added:
>  - org.apache.hadoop.hbase.coprocessor.BaseMasterObserver - stubs out full MasterObserver
interface with empty methods for convenience
>  - org.apache.hadoop.hbase.coprocessor.TestMasterObserver - tests that MasterObserver
pre/post methods are called during master operations.
> 
> In particular, please let me know if the MasterObserver method inputs and outputs are
sufficient for whatever you anticipate doing with it.  It should meet our needs for security
checks, but more input would be helpful.
> 
> 
> This addresses bug HBASE-3256.
>     http://issues.apache.org/jira/browse/HBASE-3256
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
1ffead0 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java c4fa526

>   src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/MasterCoprocessorEnvironment.java
PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/RegionCoprocessorEnvironment.java
PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java 97198ec 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/package-info.java 1b7918c 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java 18f7787 
>   src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/master/MasterServices.java 593254b 
>   src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java f71fea6 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1d48131 
>   src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java PRE-CREATION

>   src/main/resources/hbase-default.xml f1cc4ae 
>   src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationEndpoint.java 43569f1

>   src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java 902a60f

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java 8eb2787

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 5434d01

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
5f5fc9a 
>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java 3193abf

>   src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java 5be8daa 
> 
> Diff: http://review.cloudera.org/r/1321/diff
> 
> 
> Testing
> -------
> 
> Added a new test (org.apache.hadoop.hbase.coprocessor.TestMasterObserver) to cover pre/post
hook invocation.
> 
> All existing coprocessor tests still pass.
> 
> 
> Thanks,
> 
> Gary
> 
>


Mime
View raw message