hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: HBASE-2001 RegionObserver
Date Thu, 03 Jun 2010 06:28:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review123
-----------------------------------------------------------


This looks really great Andrew.


src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment680>

    What about unloading?  You remember that conversation up on irc of how loading is one
thing but unloading w/o breakage is hard prob.



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment681>

    Good



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment682>

    I love the one where fellas would be allowed intercept compactions adding their own compacting
algorithm -- but I suppose that out of scope here, or coprocessors v2? (nm... I see it later
in this patch)



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment683>

    Great doc.  Some could probably go out into package doc...



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment684>

    Make this javadoc?



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment685>

    Which table is this?



src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment686>

    Needs to be Writable?



src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
<http://review.hbase.org/r/96/#comment687>

    This is a mixin you'd use if you want to be notified about compactions, etc.?



src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
<http://review.hbase.org/r/96/#comment688>

    What would this be used for?  For CP to call out elsewhere on a table?



src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
<http://review.hbase.org/r/96/#comment689>

    Write this as LOG.warn("Failed close of table=" + table, e)?



src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
<http://review.hbase.org/r/96/#comment690>

    Is this needed?



src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
<http://review.hbase.org/r/96/#comment691>

    How does this get triggered inside the HRS?  I suppose I'll learn later down in this patch.



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.hbase.org/r/96/#comment692>

    Its always on then?



src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<http://review.hbase.org/r/96/#comment693>

    Who would want to get at this?


- stack


On 2010-05-31 22:47:25, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/96/
> -----------------------------------------------------------
> 
> (Updated 2010-05-31 22:47:25)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch is the parts of the HBASE-2001 patch which implements support for the RegionObserver
interface. This enables extension of the regionserver through stacking dynamically loaded
classes i.e. from jars on HDFS onto upcalls from HRegion. I made some improvements over the
other patch and added a test case. There are other parts of 2001 which need some thought and
some work and would not be useful without client side support. This is the part which could
be immediately useful. 
> 
> Submitted for feedback. 
> 
> Incorporates a user suggestion and Stack +1 about hooking compaction.
> 
> 
> This addresses bug HBASE-2001.
>     http://issues.apache.org/jira/browse/HBASE-2001
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java PRE-CREATION

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

>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 2413e98 
>   src/main/java/org/apache/hadoop/hbase/regionserver/MinorCompactingStoreScanner.java
71f738e 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 515b42f 
>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassloading.java PRE-CREATION

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

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

> 
> Diff: http://review.hbase.org/r/96/diff
> 
> 
> Testing
> -------
> 
> All the new unit tests plus TestHRegion pass locally.
> 
> 
> Thanks,
> 
> Andrew
> 
>


Mime
View raw message