hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: Allow Observers to completely override base function
Date Wed, 15 Dec 2010 04:48:07 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1295/#review2065
-----------------------------------------------------------

Ship it!


Looks great.  Thanks Andy!


src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
<http://review.cloudera.org/r/1295/#comment6480>

    We had discussed these returning the return type but we'll pass in an empty result instead?



src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
<http://review.cloudera.org/r/1295/#comment6481>

    so here we have to because it's a primitive.  so is there a reason not to do this elsewhere
on the pre methods?
    
    Or, this is for chaining, that's why... got it



src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
<http://review.cloudera.org/r/1295/#comment6482>

    there shouldn't be @return javadoc now, right?
    
    I think it would be noting in the javadoc that you can modify the result that is passed
in though.



src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
<http://review.cloudera.org/r/1295/#comment6483>

    if i don't do bypass/complete, and i play with this Result, what is the impact?  do we
just drop this if the flags don't get set?



src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
<http://review.cloudera.org/r/1295/#comment6484>

    looks like we drop the result.  makes sense.  not sure if we should add something to some
doc somewhere but it's clear to me now.


- Jonathan


On 2010-12-14 18:16:21, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1295/
> -----------------------------------------------------------
> 
> (Updated 2010-12-14 18:16:21)
> 
> 
> Review request for hbase, Jonathan Gray and Mingjie Lai.
> 
> 
> Summary
> -------
> 
> Currently an observer can act as a filter or translator but cannot stop a subsequent
call down to the base method for get, put, delete, etc. This patch allows observers to 1)
keep any subsequently chained observer from executing, or 2) prevent default behavior from
executing. This latter option allows a preXXX hook to completely reimplement something.
> 
> I also found and fixed some logic bugs in coprocessor framework integration in HRegion.
> 
> I will squelch the added extraneous whitespace upon commit.
> 
> 
> This addresses bug HBASE-3348.
>     http://issues.apache.org/jira/browse/HBASE-3348
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
134ed2f 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java 654b179

>   src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java 10dfff4 
>   src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java c57ca0c 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java cf9cad0 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 8248f5f 
>   src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java 345790f

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java 9ef3562

> 
> Diff: http://review.cloudera.org/r/1295/diff
> 
> 
> Testing
> -------
> 
> All coprocessor unit tests pass. No failures of other unit tests observed that might
be related to these changes.
> 
> 
> Thanks,
> 
> Andrew
> 
>


Mime
View raw message