----------------------------------------------------------- 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 > >
