hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: HBase-2001: Coprocessors: Colocate user code with regions
Date Tue, 16 Nov 2010 00:39:13 GMT


> On 2010-10-05 23:10:58, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/client/Action.java, line 30
> > <http://review.cloudera.org/r/876/diff/7/?file=14158#file14158line30>
> >
> >     I took a look at the package-info.html.  Very nice doc.  One thought though
was that the batch methods do not seem to be instrumented.  Are they?  The bulk of inserts
are done by multiput now.
> >     
> >     Maybe link to the wiki page when you say this in package-info.html '....implement
role-based access control for HBase'
> >     
> >     Fix this 'These code will be triggerd with existing...'
> >     
> >     BaseRegionObserver as the name of the class that implements BOTH Coprocessor
and RegionObserver with sensible defaults seems off... it'd make sense as the name of an implemenation
of RegionObserver but not of both.  Is there a better name to give it -- even BaseRegionObserverCoprocessor?
 Unless BaseObserver already implements Coprocessor?
> >     
> >     Should this also say that methods can be new also?  '...i.e., you can specify
new passed parameters and return types for a method. '
> >     
> >     CommandTarget is a strange name for an host of arbitrary user-designed methods.
 Can we come up w/ something more telling?   Notions that come to mind are Substrate, Platform
-- i.e. stuff you build up on.
> >     
> >     Minor.. fix '...the actually implemention class running...'
> >     
> >     Fix this '...How is the client side example of calling...'
> >     
> >     The example is missing a bit of code that would help along its illustration....
a few comments would help too.... but this is a minor criticism.  Not important.  I get the
gist (Folks interested in CP need to start with this page -- it makes grokking the code the
easier).
> >     
> >     This page would seem to indicate CPs can be chained.  Am I reading that wrong?
 (See 'Load from configuration')  Over in Gary review, he was saying on CP per region only.
> >     
> >     
> >     Usually attribute names are upper-cased.  Here we have 'Coprocessor$1' (that
$1is intentional right?)
> >     
> >     This functionality, if its working, is amazing.
> >     
> >     
> >
> 
> Mingjie Lai wrote:
>     @stack:
>     I didn't realize you posted a comment until last week, since your comments here didn't
get pushed to jira, neither emails sent to dev@hbase. 
>     
>     Thanks for your comments. I will address them very soon. But before that I'd like
to finalize the name of ``CommandTarget'':
>     
>     You said, ``CommandTarget is a strange name for an host of arbitrary user-designed
methods.  Can we come up w/ something more telling?   Notions that come to mind are Substrate,
Platform -- i.e. stuff you build up on.''
>     
>     Some of us suggested to use ``Endpoint'' instead of CommandTarget. Do you like it
better? (I'm not really good at naming stuff)
>     
>     After finalizing the name, I will make the changes to both source code and package-info.
And post a patch here.
>     
>     Thanks,
>     Mingjie
>

I'm not good at naming either.... Endpoint seems more 'generic', less loaded than 'CommandTarget'.
 If you fellas working with this stuff think that a better name then thats good by me.


- stack


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


On 2010-11-08 14:41:41, Mingjie Lai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/876/
> -----------------------------------------------------------
> 
> (Updated 2010-11-08 14:41:41)
> 
> 
> Review request for hbase, stack, Andrew Purtell, and Jonathan Gray.
> 
> 
> Summary
> -------
> 
> The diff actually contains 2 seperate patches: HBase-2001 and the one for (HBASE-2002+HBASE-2321).
The reason is that HBase-2001's CommandTarget relies on HBASE-2002 + HBASE-2321 which patches
are still under review. I have to include Gary's HBASE-2002, HBASE-2321 with this diff, since
reviewboard is so powerful :) and it disallow my diff to be based on some unchecked in patch.

> 
> Eventually the patch here should be committed after 2001 and 2321. I will make another
patch after they got checked in. 
> 
> Both HBase-2001 and the dynamic RPC stuff are quite big patches. Total number of lines
are more than 7k. I turned back and forth, but still don't have a good idea to create the
patch in order to reduce the review pain. However right now I'm putting the whole patch for
all the 3 issues. Here the list of file which are only related to coprocessor:
> 
> src/main/java/org/apache/hadoop/hbase/coprocessor/BaseEndpointCoprocessor.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorException.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
> src/main/java/org/apache/hadoop/hbase/coprocessor/package-info.java
> src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationEndpoint.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationProtocol.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
> src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java
> 
> 
> ==========================
> 
> (Here is a brief description. Please find much more details at the package-info.java
in the diff. I also post the package-info.html to https://issues.apache.org/jira/browse/HBASE-2001
as an attachment.)
> 
> 
> Coprocessors are code that runs in-process on each region server. Regions contain references
to the coprocessor implementation classes associated with them. Coprocessor classes will be
loaded either from local jars on the region server's classpath or via the HDFS classloader.
> 
> Multiple types of coprocessors are provided to provide sufficient flexibility for potential
use cases. Right now there are:
> 
> * Coprocessor: provides region lifecycle management hooks, e.g., region open/close/split/flush/compact
operations.
> * RegionObserver: provides hook for monitor table operations from client side, such as
table get/put/scan/delete, etc.
> * Endpoint: provides on demand triggers for any arbitrary function executed at a region.
One use case is column aggregation at region server.
> 
> Coprocessor:
> A coprocessor is required to implement Coprocessor interface so that coprocessor framework
can manage it internally.
> 
> Another design goal of this interface is to provide simple features for making coprocessors
useful, while exposing no more internal state or control actions of the region server than
necessary and not exposing them directly. 
> 
> RegionObserver
> If the coprocessor implements the RegionObserver interface it can observe and mediate
client actions on the region. 
> 
> Endpoint:
> Coprocessor and RegionObserver provide certain hooks for injecting user code running
at each region. These code will be triggerd with existing HTable and HBaseAdmin operations
at the certain hook points.
> 
> Through Endpoint and dynamic RPC protocol, you can define your own interface communicated
between client and region server, i.e., you can create a new method, specify passed parameters
and return types for the method. And the new Endpoint methods can be triggered by calling
client side dynamic RPC functions -- HTable.exec(...). 
> 
> Coprocess loading
> A customized coprocessor can be loaded by two different ways, by configuration, or by
HTableDescriptor for a newly created table.
> 
> (Currently we don't really have an on demand coprocessor loading machanism for opened
regions. However it should be easy to create a dedicated CommandTarget for coprocessor loading)

> 
> 
> This addresses bug HBase-2001.
>     http://issues.apache.org/jira/browse/HBase-2001
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/client/Action.java 556ea81 
>   src/main/java/org/apache/hadoop/hbase/client/HConnection.java 3235a9b 
>   src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 1748a4f 
>   src/main/java/org/apache/hadoop/hbase/client/HTable.java 69dc916 
>   src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java ade3e21 
>   src/main/java/org/apache/hadoop/hbase/client/MultiAction.java c6ea838 
>   src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java 1ac7671 
>   src/main/java/org/apache/hadoop/hbase/client/coprocessor/Batch.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/client/coprocessor/Exec.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/client/coprocessor/ExecResult.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/client/coprocessor/package-info.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/BaseEndpointCoprocessor.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java
PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/Coprocessor.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorException.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/coprocessor/package-info.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 30dd877 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorProtocol.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java f8d8af7 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 4f4828b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java e4c356d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java cf30cf9 
>   src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java 209c291 
>   src/main/java/org/apache/hadoop/hbase/regionserver/CoprocessorHost.java PRE-CREATION

>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java dd2955d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ab183e5 
>   src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java 1309f93

>   src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java 1bcde8c 
>   src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java ace7997

>   src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 5f05079 
>   src/main/resources/hbase-default.xml 5fafe65 
>   src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationEndpoint.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationProtocol.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java PRE-CREATION

>   src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorEndpoint.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

>   src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java PRE-CREATION

> 
> Diff: http://review.cloudera.org/r/876/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mingjie
> 
>


Mime
View raw message