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