> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > pom.xml, line 480
> > <http://review.cloudera.org/r/406/diff/2/?file=3873#file3873line480>
> >
> >     is this build up somewhere? or you have to mvn install it locally?

Currently you have to mvn install it locally (from 
http://github.com/apurtell/hadoop-common), but I'll add a public mvn repo with 
it to ease building/testing.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 812
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line812>
> >
> >     typo, but maybe also in hadoop

Copied from Hadoop, but private so seems safe to fix.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 437
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line437>
> >
> >     hrm, not sure if this was just a copy from hadoop, but you added the 
> > argument e but doesn't seem to be used?

Copied from Hadoop, will clean up.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java, line 266
> > <http://review.cloudera.org/r/406/diff/2/?file=3891#file3891line266>
> >
> >     maybe this should be part of addRegionServer?

Makes sense, I can clean up the doAs() part.  I did want to keep the test user 
creation separate though, since it's testing specific and LocalHBaseCluster is 
used in standalone mode.


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 1337
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line1337>
> >
> >     these are HBase todos? isn't the doAs important so that further RPCs 
> > sent by the handling of this call get done as the user, etc?

Yes, these are HBase TODO's.  Will clarify the comments.  We definitely need to 
maintain the caller's UGI in the execution context for servicing this RPC and 
would probably need to pass it through for any additional HBase RPCs that it 
generates.  That'll be one of the primary parts of the RBAC implementation 
(lumped under HBASE-1697).

But how this maps to HBase interaction as client of HDFS is still up in the 
air.  The phase one plan was to have HBase run as it's own server principal 
from the HDFS perspective, maintaining ownership of all the individual store 
files.  There's been some thinking about how we provide ownership (and 
isolation) down to the HDFS layer as a phase two, but that requires working 
around the HDFS lack of ACLs and has some trade-offs for the overall security 
picture (insulates you from defects in the HBase implementation but introduces 
need to pass around, and possibly store, client credentials).


> On 2010-07-29 16:49:36, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 127
> > <http://review.cloudera.org/r/406/diff/2/?file=3878#file3878line127>
> >
> >     maybe not a good idea to default the max response size to 1M in hbase?

Good point.  This impacts the HBaseServer.Handler output buffer size (if 
exceeded it will re-instantiate the buffer with the initial size again), so 
trades off used heap vs. ByteArrayOutputStream instantiations.  And for large 
numbers of handlers of course this can consume significant heap.

Thoughts on a more appropriate size for standard HBase requests?  64k? 128k?


- Gary


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


On 2010-07-29 12:40:06, Gary Helmling wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/406/
> -----------------------------------------------------------
> 
> (Updated 2010-07-29 12:40:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch ports over the secure Hadoop RPC changes from the latest Yahoo 
> 0.20 based branch (yahoo-hadoop-0.20.104).  This patch is produced against 
> HBase trunk, but is targeted as the first step in a "security" feature branch 
> for a full role-based access control implementation (HBASE-1697).
> 
> RPC Changes
> --------------------
> The primary changes are updates from the classes:
> org.apache.hadoop.ipc.Client -> org.apache.hadoop.hbase.ipc.HBaseClient
> org.apache.hadoop.ipc.RPC -> org.apache.hadoop.hbase.ipc.HBaseRPC
> org.apache.hadoop.ipc.Server -> org.apache.hadoop.hbase.ipc.HBaseServer
> 
> The new classes were also ported:
> org.apache.hadoop.hbase.security.HBaseSaslRpcClient
> org.apache.hadoop.hbase.security.HBaseSaslRpcServer
> 
> Due to type dependencies on the Hadoop RPC classes, the original Hadoop 
> SaslRpc* classes could not be used.
> 
> The RPC port provides client authentication via Kerberos, and SASL 
> negotiation of client server connections for mutual authentication and 
> optionally encryption, so it also provides the authentication functionality 
> for HBASE-2016.  The ported RPC code contains dependencies on other classes 
> in secure Hadoop/Hadoop trunk, preventing it from currently running on 0.20 
> branches missing the security changes.
> 
> Process Authentication
> ---------------------------
> The HMaster and HRegionServer processes have been updated to allow 
> configuration of the Kerberos principals used to run the processes.  The new 
> configuration parameters are:
> 
> * hbase.master.keytab.file - Path to the keytab file containing the master 
> principal's credentials
> * hbase.master.kerberos.principal - Kerberos principal name used to login the 
> HMaster process
> * hbase.master.kerberos.https.principal - Kerberos principal name used to 
> login the HMaster info server
> * hbase.regionserver.keytab.file - Path to the keytab file containing the 
> region server's credentials
> * hbase.regionserver.kerberos.principal - Kerberos principal name used to 
> login the HRegionServer process
> * hbase.regionserver.kerberos.https.principal - Kerberos principal name used 
> to login the HRegionServer info server
> 
> The new class org.apache.hadoop.hbase.security.HBasePolicyProvider and new 
> file conf/hadoop-policy.xml allow restriction of the users and groups 
> permitting to utilize each of the RPC protocol interfaces (HMasterInterface, 
> HMasterRegionInterface, HRegionInterface).
> 
> Testing Updates
> --------------------
> Parts of the test code (org.apache.hadoop.hbase.HBaseTestingUtility and 
> org.apache.hadoop.hbase.MiniHBaseCluster) were directly using the internal 
> Hadoop UnixUserGroupInformation class to manipulate process ownership for 
> testing.  These have been updated to use UserGroupInformation.doAs() instead.
> 
> 
> This addresses bugs HBASE-2016 and HBASE-2742.
>     http://issues.apache.org/jira/browse/HBASE-2016
>     http://issues.apache.org/jira/browse/HBASE-2742
> 
> 
> Diffs
> -----
> 
>   conf/hadoop-policy.xml PRE-CREATION 
>   pom.xml 2d3d75a 
>   src/main/java/org/apache/hadoop/hbase/ipc/ConnectionHeader.java 
> PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 2b5eeb6 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java 9873172 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseRpcMetrics.java d88c12d 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterInterface.java bd48a4b 
>   src/main/java/org/apache/hadoop/hbase/ipc/HMasterRegionInterface.java 
> 71a0447 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1157fe1 
>   src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/master/HMaster.java e4bd30d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 
> 6a54736 
>   src/main/java/org/apache/hadoop/hbase/security/HBasePolicyProvider.java 
> PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java 
> PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcServer.java 
> PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 280b91d 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 4d09fe9 
>   src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 9c49e36 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 0b47975 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 
> c982662 
> 
> Diff: http://review.cloudera.org/r/406/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gary
> 
>

Reply via email to