> On 2010-12-16 17:09:44, Andrew Purtell wrote: > > src/main/java/org/apache/hadoop/hbase/coprocessor/BaseEndpointCoprocessor.java, > > line 66 > > <http://review.cloudera.org/r/1306/diff/1/?file=18379#file18379line66> > > > > What are these arguments about?
Those are: - String protocol - long clientVersion from org.apache.hadoop.ipc.VersionedProtocol. Will fix these up. > On 2010-12-16 17:09:44, Andrew Purtell wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java, > > line 289 > > <http://review.cloudera.org/r/1306/diff/1/?file=18383#file18383line289> > > > > Should be a WARN? Yeah, agree. Will fix. > On 2010-12-16 17:09:44, Andrew Purtell wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java, > > line 305 > > <http://review.cloudera.org/r/1306/diff/1/?file=18383#file18383line305> > > > > Should be a WARN? Yeah, will fix. > On 2010-12-16 17:09:44, Andrew Purtell wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java, > > line 385 > > <http://review.cloudera.org/r/1306/diff/1/?file=18383#file18383line385> > > > > Since you are committing a change set in this area, Ryan suggested no > > need for AtomicBoolean here, could just be plain volatile boolean. I think > > that's right. Ok will change this to a volatile boolean and repost. - Gary ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/1306/#review2102 ----------------------------------------------------------- On 2010-12-16 16:26:20, Gary Helmling wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/1306/ > ----------------------------------------------------------- > > (Updated 2010-12-16 16:26:20) > > > Review request for hbase, stack and Andrew Purtell. > > > Summary > ------- > > This patch adds explicit start() and stop() methods for lifecycle management > to the Coprocessor interface and refactors some of the > Coprocessor/RegionObserver distinction, moving the region-related pre/post > hooks that were previously in Coprocessor to RegionObserver. > > Coprocessor is now the base interface, containing only: > - start() > - stop() > - Priority enum > - State enum > > RegionObserver extends Coprocessor, and now contains the additional pre/post > hooks, moved from Coprocessor: > - pre/postOpen > - pre/postClose > - pre/postFlush > - pre/postCompact > - pre/postSplit > > This will allow cleaner extension in the future, to allow addition of a > MasterObserver interface, for example. > > As shown above, I've also added a new Coprocessor.State enum consisting of > the states: > UNINSTALLED -> INSTALLED -> STARTING -> ACTIVE -> STOPPING -> STOPPED > > However, the UNINSTALLED/INSTALLED distinction is not particularly useful at > the moment. I'd appreciate other feedback on what's necessary here. The > current handling could make do with: > UNINSTALLED -> STARTING -> ACTIVE -> STOPPING -> UNINSTALLED (4 total states) > > However, the UNINSTALLED/INSTALLED distinction may be useful if we want to > add class level initialization in the future... > > > This addresses bug HBASE-3260. > http://issues.apache.org/jira/browse/HBASE-3260 > > > Diffs > ----- > > > src/main/java/org/apache/hadoop/hbase/coprocessor/BaseEndpointCoprocessor.java > b81a465 > > src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java > f022598 > src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java 7ea1c5e > src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java > 1792290 > src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java > f028525 > src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java > 3db4c36 > > src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java > 81cb75d > > Diff: http://review.cloudera.org/r/1306/diff > > > Testing > ------- > > Added tests for start() and stop() method invocation in > org.apache.hadoop.hbase.coprocessor.TestCoprocessorInterface > > The existing TestCoprocessorEndpoint, TestCoprocessorInterface, > TestRegionObserverInterface, TestRegionObserverStacking tests continue to > work. I'm not seeing any new failures in the rest of the tests, but > TestReplication is timing out for me, preventing all tests from executing. > > > Thanks, > > Gary > >
