hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gary Helmling" <ghelml...@gmail.com>
Subject Re: Review Request: HBASE-3256: Coprocessors: Coprocessor host and observer for HMaster
Date Tue, 21 Dec 2010 05:35:56 GMT

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



src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
<http://review.cloudera.org/r/1321/#comment6621>

    It actually means "region".  That conf key is only used for the system coprocessors loaded
on regions.
    
    I'll change the name (and config property).



src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
<http://review.cloudera.org/r/1321/#comment6622>

    Yes, I'm not sure what the original intent was here, obtaining the CP without the full
package name?
    
    Maybe getClass().getSimpleName().equals() would be better?



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/1321/#comment6623>

    This is the result of env.shouldBypass(), in order to allow a MasterObserver to bypass
the normal balance() processing.



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/1321/#comment6624>

    Right, that's the only way to modify the input.  Will change.



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/1321/#comment6628>

    I can add in env.shouldBypass() handling here to allow overriding.  Combined with access
to ServerManager through MasterServices, this should allow custom assignment policies.



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/1321/#comment6629>

    Yes, will add in env.shouldBypass() handling here too.


- Gary


On 2010-12-20 18:04:33, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1321/
> -----------------------------------------------------------
> 
> (Updated 2010-12-20 18:04:33)
> 
> 
> 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/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/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/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