> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 131
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line131>
> >
> >     What is purpose of this?

Coprocessors may want to change their behavior based on different minor 
versions. 


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java, line 189
> > <http://review.hbase.org/r/96/diff/4/?file=892#file892line189>
> >
> >     Split decisions will not be made post-compaction as they are now after 
> > HBASE-2375 goes in.  That decision will actually be made at flush time, 
> > most likely post-flush though we'll know at the start whether it will end 
> > up needing to split.

Then this signal should shift to the flush upcall.


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java, line 
> > 32
> > <http://review.hbase.org/r/96/diff/4/?file=893#file893line32>
> >
> >     So a coprocessor implementation would potentially implement Coprocessor 
> > and RegionObserver?  Notifications of higher level events happen through 
> > Coprocessor, this is for lower level hooks?  Maybe a bit more detail in 
> > class comment to describe difference between the two interfaces.

RegionObserver used to have a cleaner distinction from Coprocessor. Initially 
Coprocessor hooked internal region housekeeping; meanwhile RegionObserver 
wrapped HRegionInterface. 


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java, line 
> > 33
> > <http://review.hbase.org/r/96/diff/4/?file=893#file893line33>
> >
> >     This makes sense now reading the rest of the code.  But it seems that 
> > the Coprocessor is in fact the "observer" that just gets notified of 
> > actions while this observer is actually the "processor" that can manipulate 
> > stuff?

An interesting idea to consider renaming the interfaces for clarity.


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java, line 
> > 49
> > <http://review.hbase.org/r/96/diff/4/?file=893#file893line49>
> >
> >     And descending timestamp

Got it.


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java, line 
> > 87
> > <http://review.hbase.org/r/96/diff/4/?file=893#file893line87>
> >
> >     Gets are called after the Get is performed, Puts and Deletes are called 
> > before, correct?
> >     
> >     Would there be a use case for pre-Get hook?  Just wondering.

I wanted for the interface to be able to munge the result of the Get and only 
was considering one "onGet". But for sure we could do "onGet" and 
"onGetResult". 


> On 2010-06-03 11:19:47, Jonathan Gray wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 694
> > <http://review.hbase.org/r/96/diff/4/?file=895#file895line694>
> >
> >     Javadoc says it's called after the split happens but before report to 
> > master.  Seems that this happens once we create the new HRegions but before 
> > we actually do the swap.  What exactly would/could a coprocessor be doing 
> > in this window?
> >     
> >     One thing to be aware of is the master changes coming are going to make 
> > a split run entirely on the RS including the edits to META, closing of the 
> > parent, and opening of the children.  Where in that process would this hook 
> > make sense?

This is so a CP on a parent can know in advance that daughters from a split are 
about to come on line, and their associated CPs are about to be initialized and 
become operational. I don't know how useful this would be but it seemed a 
reasonable part of the region lifecycle to intercept. 


- Andrew


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


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