> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 140
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line140>
> >
> >     Which table is this?

Any table. The idea is the coprocessor can create any private tables it needs 
to implement its functionality.


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 27
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line27>
> >
> >     What about unloading?  You remember that conversation up on irc of how 
> > loading is one thing but unloading w/o breakage is hard prob.

I'm not trying to tackle unloading yet.

However, classes are strongly bound to their classloader. We do instantiate a 
classloader each time to load a coprocessor and we don't hold a reference to 
the classloader. It is my understanding that when there are no more references 
to the classes (no live objects), they and the classloader will be garbage 
collected, though the JVM spec does not guarantee this the Sun JVM will do 
this. Creating a new classloader and asking for the class again, presumably 
from an updated jar, should load the new class -- a unit test can verify. 

To help insure old classes don't leak via live objects hanging around, we could 
consider a cooperative lifecycle management scheme like that used by OSGi: 
http://en.wikipedia.org/wiki/OSGi. 


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 150
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line150>
> >
> >     Needs to be Writable?

No.


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java, line 
> > 29
> > <http://review.hbase.org/r/96/diff/4/?file=893#file893line29>
> >
> >     This is a mixin you'd use if you want to be notified about compactions, 
> > etc.?

This is for translating values found in a flush file into new values in the new 
storefile being built by the compaction, or for dropping values. 


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java, 
> > line 85
> > <http://review.hbase.org/r/96/diff/4/?file=894#file894line85>
> >
> >     What would this be used for?  For CP to call out elsewhere on a table?

The idea is the coprocessor can create any private tables it needs to implement 
its functionality. But we want to mediate that, add access control, clean up 
references when/if the cp is terminated (and perhaps unloaded).


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java, 
> > line 237
> > <http://review.hbase.org/r/96/diff/4/?file=894#file894line237>
> >
> >     Is this needed?

Why not.


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 245
> > <http://review.hbase.org/r/96/diff/4/?file=895#file895line245>
> >
> >     Its always on then?

Yes, otherwise I have to wrap all calls to the cp host in HRegion with "if 
(coprocessorHost != null) then", including the inner loops of the major and 
minor compactors. 


> On 2010-06-02 23:28:50, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2885
> > <http://review.hbase.org/r/96/diff/4/?file=895#file895line2885>
> >
> >     Who would want to get at this?

Tests. So probably this does not have to be public.


- Andrew


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


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

Reply via email to