hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Purtell" <apurt...@apache.org>
Subject Re: Review Request: HBASE-2001 RegionObserver
Date Fri, 04 Jun 2010 04:57:40 GMT


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


Mime
View raw message