You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Andrew Purtell <ap...@apache.org> on 2010/06/01 07:30:41 UTC
Re: Review Request: HBASE-2001 RegionObserver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/
-----------------------------------------------------------
(Updated 2010-05-31 22:30:41.618893)
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
Re: Review Request: HBASE-2001 RegionObserver
Posted by Andrew Purtell <ap...@apache.org>.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > this seems like a reasonable framework, but I'd rather see this stay around as a branch until there is at least one or two actual things using it for a real purpose. Otherwise I think we'll end up shipping an API that we later realize doesn't work for real apps. What do you think?
>
> stack wrote:
> This seems like a good idea when dev'ing an Interface. After the 3rd implemenation you'll have some confidence in your Interface.
Ok, I'll take all the comments below and incorporate the feedback in a new version and commit it on a feature branch that will track trunk. I can use git to manage the merging and just push snapshots into SVN. Will set up a project on our Hudson to crunch tests for it.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review121
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by st...@duboce.net.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > this seems like a reasonable framework, but I'd rather see this stay around as a branch until there is at least one or two actual things using it for a real purpose. Otherwise I think we'll end up shipping an API that we later realize doesn't work for real apps. What do you think?
This seems like a good idea when dev'ing an Interface. After the 3rd implemenation you'll have some confidence in your Interface.
- stack
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review121
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by Andrew Purtell <ap...@apache.org>.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 53
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line53>
> >
> > before or after reporting?
Should be just before. Will double check.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review121
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by Andrew Purtell <ap...@apache.org>.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassloading.java, line 40
> > <http://review.hbase.org/r/96/diff/4/?file=898#file898line40>
> >
> > woah, can we add something to the build to build this jar as a test resource, or something?
Yes.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java, line 43
> > <http://review.hbase.org/r/96/diff/4/?file=899#file899line43>
> >
> > you could use mockito verification to do this, probably would be simpler
Thanks this would be a good opportunity to learn mockito.
> On 2010-06-02 17:24:34, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 149
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line149>
> >
> > this map-like interface is somewhat confusing - what's the purpose of it?
This is an environment space, like unix process env vars, shared among all threads of the coprocessor (which get a reference to the environment). Useful for the mapreduce stuff not included in this patch. For example, rather than sum or average using intermediates, update AtomicLongs instead.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review121
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/#review121
-----------------------------------------------------------
this seems like a reasonable framework, but I'd rather see this stay around as a branch until there is at least one or two actual things using it for a real purpose. Otherwise I think we'll end up shipping an API that we later realize doesn't work for real apps. What do you think?
src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment676>
before or after reporting?
src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
<http://review.hbase.org/r/96/#comment677>
this map-like interface is somewhat confusing - what's the purpose of it?
src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassloading.java
<http://review.hbase.org/r/96/#comment678>
woah, can we add something to the build to build this jar as a test resource, or something?
src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java
<http://review.hbase.org/r/96/#comment679>
you could use mockito verification to do this, probably would be simpler
- Todd
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by Andrew Purtell <ap...@apache.org>.
> 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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by st...@duboce.net.
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2001 RegionObserver
Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/96/
-----------------------------------------------------------
(Updated 2010-05-31 22:47:25.650165)
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 (updated)
-----
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