You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by sd...@apache.org on 2015/04/21 10:48:50 UTC

incubator-sentry git commit: SENTRY-296: Sentry Service Client does not allow for connection pooling (Dapeng Sun and Colin Ma, reviewed by Prasad Mujumdar)

Repository: incubator-sentry
Updated Branches:
  refs/heads/master 244876100 -> 7ae7fc375


SENTRY-296: Sentry Service Client does not allow for connection pooling (Dapeng Sun and Colin Ma, reviewed by Prasad Mujumdar)


Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/7ae7fc37
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/7ae7fc37
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/7ae7fc37

Branch: refs/heads/master
Commit: 7ae7fc375e3458e77f55e5e4feff45ef1f9a18e0
Parents: 2448761
Author: Sun Dapeng <sd...@apache.org>
Authored: Tue Apr 21 16:17:43 2015 +0800
Committer: Sun Dapeng <sd...@apache.org>
Committed: Tue Apr 21 16:17:43 2015 +0800

----------------------------------------------------------------------
 pom.xml                                         |   6 +
 sentry-provider/sentry-provider-db/pom.xml      |   4 +
 .../thrift/HAClientInvocationHandler.java       |  18 +--
 .../thrift/PoolClientInvocationHandler.java     | 154 +++++++++++++++++++
 .../thrift/SentryClientInvocationHandler.java   |  54 +++++++
 .../thrift/SentryServiceClientFactory.java      |  12 +-
 .../thrift/SentryServiceClientPoolFactory.java  |  78 ++++++++++
 .../sentry/service/thrift/ServiceConstants.java |  19 ++-
 ...estSentryServerForPoolHAWithoutKerberos.java |  36 +++++
 .../TestSentryServerForPoolWithoutKerberos.java |  36 +++++
 .../thrift/TestSentryServiceClientPool.java     | 113 ++++++++++++++
 .../TestSentryServiceForPoolHAWithKerberos.java |  36 +++++
 .../TestSentryServiceForPoolWithKerberos.java   |  36 +++++
 .../thrift/SentryServiceIntegrationBase.java    |  12 +-
 14 files changed, 600 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 90ecea1..863f70c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -93,6 +93,7 @@ limitations under the License.
     <easymock.version>3.0</easymock.version>
     <objenesis.version>1.2</objenesis.version>
     <cglib.version>2.2</cglib.version>
+    <commons-pool2.version>2.2</commons-pool2.version>
   </properties>
 
   <dependencyManagement>
@@ -525,6 +526,11 @@ limitations under the License.
         <artifactId>cglib-nodep</artifactId>
         <version>${cglib.version}</version>
       </dependency>
+      <dependency>
+        <groupId>org.apache.commons</groupId>
+        <artifactId>commons-pool2</artifactId>
+        <version>${commons-pool2.version}</version>
+      </dependency>
     </dependencies>
   </dependencyManagement>
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/pom.xml
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/pom.xml b/sentry-provider/sentry-provider-db/pom.xml
index 9c2fc81..7dd40b8 100644
--- a/sentry-provider/sentry-provider-db/pom.xml
+++ b/sentry-provider/sentry-provider-db/pom.xml
@@ -188,6 +188,10 @@ limitations under the License.
       <groupId>org.apache.curator</groupId>
       <artifactId>curator-test</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-pool2</artifactId>
+    </dependency>
   </dependencies>
 
   <build>

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
index 4947ad1..377e934 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
@@ -18,7 +18,6 @@
 package org.apache.sentry.service.thrift;
 
 import java.io.IOException;
-import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.InetSocketAddress;
@@ -37,7 +36,7 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
 
-public class HAClientInvocationHandler implements InvocationHandler {
+public class HAClientInvocationHandler extends SentryClientInvocationHandler {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(HAClientInvocationHandler.class);
 
@@ -55,7 +54,7 @@ public class HAClientInvocationHandler implements InvocationHandler {
   }
 
   @Override
-  public Object invoke(Object proxy, Method method, Object[] args) throws
+  public Object invokeImpl(Object proxy, Method method, Object[] args) throws
       SentryUserException {
     Object result = null;
     while (true) {
@@ -83,12 +82,6 @@ public class HAClientInvocationHandler implements InvocationHandler {
           }
         }
       } catch (IOException e1) {
-        // close() doesn't throw exception we supress that in case of connection
-        // loss. Changing SentryPolicyServiceClient#close() to throw an
-        // exception would be a backward incompatible change for Sentry clients.
-        if ("close".equals(method.getName())) {
-          return null;
-        }
         throw new SentryUserException("Error connecting to sentry service "
             + e1.getMessage(), e1);
       }
@@ -138,4 +131,11 @@ public class HAClientInvocationHandler implements InvocationHandler {
           ServerConfig.PRINCIPAL + " : " + serverPrincipal + " should contain " + SecurityUtil.HOSTNAME_PATTERN);
     }
   }
+
+  @Override
+  public void close() {
+    if (client != null) {
+      client.close();
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
new file mode 100644
index 0000000..1e7a789
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sentry.service.thrift;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+import org.apache.commons.pool2.PooledObjectFactory;
+import org.apache.commons.pool2.impl.AbandonedConfig;
+import org.apache.commons.pool2.impl.GenericObjectPool;
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.sentry.SentryUserException;
+import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClient;
+import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
+import org.apache.thrift.transport.TTransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The PoolClientInvocationHandler is a proxy class for handling thrift call. For every thrift call,
+ * get the instance of SentryPolicyServiceBaseClient from the commons-pool, and return the instance
+ * to the commons-pool after complete the call. For any exception with the call, discard the
+ * instance and create a new one added to the commons-pool. Then, get the instance and do the call
+ * again. For the thread safe, the commons-pool will manage the connection pool, and every thread
+ * can get the connection by borrowObject() and return the connection to the pool by returnObject().
+ */
+
+public class PoolClientInvocationHandler extends SentryClientInvocationHandler {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(PoolClientInvocationHandler.class);
+
+  private final Configuration conf;
+  private PooledObjectFactory<SentryPolicyServiceClient> poolFactory;
+  private GenericObjectPool<SentryPolicyServiceClient> pool;
+  private GenericObjectPoolConfig poolConfig;
+  private int connectionRetryTotal;
+
+  private static final String POOL_EXCEPTION_MESSAGE = "Pool exception occured ";
+
+  public PoolClientInvocationHandler(Configuration conf) throws Exception {
+    this.conf = conf;
+    readConfiguration();
+    poolFactory = new SentryServiceClientPoolFactory(conf);
+    pool = new GenericObjectPool<SentryPolicyServiceClient>(poolFactory, poolConfig, new AbandonedConfig());
+  }
+
+  @Override
+  public Object invokeImpl(Object proxy, Method method, Object[] args) throws Exception {
+    int retryCount = 0;
+    Object result = null;
+    while (retryCount < connectionRetryTotal) {
+      try {
+        // The wapper here is for the retry of thrift call, the default retry number is 3.
+        result = invokeFromPool(proxy, method, args);
+        break;
+      } catch (TTransportException e) {
+        // TTransportException means there has connection problem, create a new connection and try
+        // again. Get the lock of pool and add new connection.
+        synchronized (pool) {
+          // If there has room, create new instance and add it to the commons-pool, this instance
+          // will be back first from the commons-pool because the configuration is LIFO.
+          if (pool.getNumIdle() + pool.getNumActive() < pool.getMaxTotal()) {
+            pool.addObject();
+          }
+        }
+        // Increase the retry num, and throw the exception if can't retry again.
+        retryCount++;
+        if (retryCount == connectionRetryTotal) {
+          throw new SentryUserException(e.getMessage(), e);
+        }
+      }
+    }
+    return result;
+  }
+
+  private Object invokeFromPool(Object proxy, Method method, Object[] args) throws Exception {
+    Object result = null;
+    SentryPolicyServiceClient client;
+    try {
+      // get the connection from the pool, don't know if the connection is broken.
+      client = pool.borrowObject();
+    } catch (Exception e) {
+      LOGGER.debug(POOL_EXCEPTION_MESSAGE, e);
+      throw new SentryUserException(e.getMessage(), e);
+    }
+    try {
+      // do the thrift call
+      result = method.invoke(client, args);
+    } catch (InvocationTargetException e) {
+      // Get the target exception, check if SentryUserException or TTransportException is wrapped.
+      // TTransportException means there has connection problem with the pool.
+      Throwable targetException = e.getCause();
+      if (targetException != null && targetException instanceof SentryUserException) {
+        Throwable sentryTargetException = targetException.getCause();
+        // If there has connection problem, eg, invalid connection if the service restarted,
+        // sentryTargetException instanceof TTransportException = true.
+        if (sentryTargetException != null && sentryTargetException instanceof TTransportException) {
+          // If the exception is caused by connection problem, destroy the instance and
+          // remove it from the commons-pool. Throw the TTransportException for reconnect.
+          pool.invalidateObject(client);
+          throw new TTransportException(sentryTargetException);
+        }
+        // The exception is thrown by thrift call, eg, SentryAccessDeniedException.
+        throw (SentryUserException) targetException;
+      }
+      throw e;
+    } finally{
+      try {
+        // return the instance to commons-pool
+        pool.returnObject(client);
+      } catch (Exception e) {
+        LOGGER.error(POOL_EXCEPTION_MESSAGE, e);
+        throw e;
+      }
+    }
+    return result;
+  }
+
+  @Override
+  public void close() {
+    try {
+      pool.close();
+    } catch (Exception e) {
+      LOGGER.debug(POOL_EXCEPTION_MESSAGE, e);
+    }
+  }
+
+  private void readConfiguration() {
+    poolConfig = new GenericObjectPoolConfig();
+    // config the pool size for commons-pool
+    poolConfig.setMaxTotal(conf.getInt(ClientConfig.SENTRY_POOL_MAX_TOTAL, ClientConfig.SENTRY_POOL_MAX_TOTAL_DEFAULT));
+    poolConfig.setMinIdle(conf.getInt(ClientConfig.SENTRY_POOL_MIN_IDLE, ClientConfig.SENTRY_POOL_MIN_IDLE_DEFAULT));
+    poolConfig.setMaxIdle(conf.getInt(ClientConfig.SENTRY_POOL_MAX_IDLE, ClientConfig.SENTRY_POOL_MAX_IDLE_DEFAULT));
+    // get the retry number for reconnecting service
+    connectionRetryTotal = conf.getInt(ClientConfig.SENTRY_POOL_RETRY_TOTAL,
+        ClientConfig.SENTRY_POOL_RETRY_TOTAL_DEFAULT);
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java
new file mode 100644
index 0000000..a41be7f
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java
@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sentry.service.thrift;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+
+/**
+ * SentryClientInvocationHandler is the base interface for all the InvocationHandler in SENTRY
+ */
+public abstract class SentryClientInvocationHandler implements InvocationHandler {
+
+  /**
+   * Close the InvocationHandler: An InvocationHandler may create some contexts,
+   * these contexts should be close when the method "close()" of client be called.
+   */
+  @Override
+  public final Object invoke(Object proxy, Method method, Object[] args) throws Exception {
+    // close() doesn't throw exception we supress that in case of connection
+    // loss. Changing SentryPolicyServiceClient#close() to throw an
+    // exception would be a backward incompatible change for Sentry clients.
+    if ("close".equals(method.getName()) && null == args) {
+      close();
+      return null;
+    }
+    return invokeImpl(proxy, method, args);
+  }
+
+  /**
+   * Subclass should implement this method for special function
+   */
+  public abstract Object invokeImpl(Object proxy, Method method, Object[] args) throws Exception;
+
+  /**
+   * An abstract method "close", an invocationHandler should close its contexts at here.
+   */
+  public abstract void close();
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
index 574f23c..09fe42e 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
@@ -24,7 +24,7 @@ import org.apache.hadoop.conf.Configuration;
 
 import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClient;
 import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClientDefaultImpl;
-import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
+import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
 
 public class SentryServiceClientFactory {
 
@@ -32,8 +32,14 @@ public class SentryServiceClientFactory {
   }
 
   public static SentryPolicyServiceClient create(Configuration conf) throws Exception {
-    boolean haEnabled = conf.getBoolean(ServerConfig.SENTRY_HA_ENABLED, false);
-    if (haEnabled) {
+    boolean haEnabled = conf.getBoolean(ClientConfig.SERVER_HA_ENABLED, false);
+    boolean pooled = conf.getBoolean(ClientConfig.SENTRY_POOL_ENABLED, false);
+    if (pooled) {
+      return (SentryPolicyServiceClient) Proxy
+          .newProxyInstance(SentryPolicyServiceClientDefaultImpl.class.getClassLoader(),
+              SentryPolicyServiceClientDefaultImpl.class.getInterfaces(),
+              new PoolClientInvocationHandler(conf));
+    } else if (haEnabled) {
       return (SentryPolicyServiceClient) Proxy
           .newProxyInstance(SentryPolicyServiceClientDefaultImpl.class.getClassLoader(),
               SentryPolicyServiceClientDefaultImpl.class.getInterfaces(),

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java
new file mode 100644
index 0000000..3a38b24
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.service.thrift;
+
+import java.lang.reflect.Proxy;
+
+import org.apache.commons.pool2.BasePooledObjectFactory;
+import org.apache.commons.pool2.PooledObject;
+import org.apache.commons.pool2.impl.DefaultPooledObject;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClient;
+import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClientDefaultImpl;
+import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * SentryServiceClientPoolFactory is for connection pool to manage the object. Implement the related
+ * method to create object, destroy object and wrap object.
+ */
+
+public class SentryServiceClientPoolFactory extends BasePooledObjectFactory<SentryPolicyServiceClient> {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(SentryServiceClientPoolFactory.class);
+
+  private Configuration conf;
+
+  public SentryServiceClientPoolFactory(Configuration conf) {
+    this.conf = conf;
+  }
+
+  @Override
+  public SentryPolicyServiceClient create() throws Exception {
+    LOGGER.debug("Creating Sentry Service Client...");
+    boolean haEnabled = conf.getBoolean(ClientConfig.SERVER_HA_ENABLED, false);
+    if (haEnabled) {
+      return (SentryPolicyServiceClient) Proxy
+          .newProxyInstance(SentryPolicyServiceClientDefaultImpl.class.getClassLoader(),
+              SentryPolicyServiceClientDefaultImpl.class.getInterfaces(),
+              new HAClientInvocationHandler(conf));
+    } else {
+      return new SentryPolicyServiceClientDefaultImpl(conf);
+    }
+  }
+
+  @Override
+  public PooledObject<SentryPolicyServiceClient> wrap(SentryPolicyServiceClient client) {
+    return new DefaultPooledObject<SentryPolicyServiceClient>(client);
+  }
+
+  @Override
+  public void destroyObject(PooledObject<SentryPolicyServiceClient> pooledObject) {
+    SentryPolicyServiceClient client = pooledObject.getObject();
+    LOGGER.debug("Destroying Sentry Service Client: " + client);
+    if (client != null) {
+      // The close() of TSocket or TSaslClientTransport is called actually, and there has no
+      // exception even there has some problems, eg, the client is closed already.
+      // The close here is just try to close the socket and the client will be destroyed soon.
+      client.close();
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
index c8f7450..54dbac5 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
@@ -22,9 +22,10 @@ import java.util.Map;
 
 import javax.security.sasl.Sasl;
 
+import org.apache.sentry.provider.db.service.thrift.SentryMetrics;
+
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
-import org.apache.sentry.provider.db.service.thrift.SentryMetrics;
 
 public class ServiceConstants {
 
@@ -182,6 +183,22 @@ public class ServiceConstants {
     public static final String SENTRY_HA_ZOOKEEPER_NAMESPACE = ServerConfig.SENTRY_HA_ZOOKEEPER_NAMESPACE;
     public static final String SERVER_HA_ZOOKEEPER_NAMESPACE_DEFAULT = ServerConfig.SENTRY_HA_ZOOKEEPER_NAMESPACE_DEFAULT;
 
+    // connection pool configuration
+    public static final String SENTRY_POOL_ENABLED = "sentry.service.client.connection.pool.enabled";
+    public static final boolean SENTRY_POOL_ENABLED_DEFAULT = false;
+
+    // commons-pool configuration for pool size
+    public static final String SENTRY_POOL_MAX_TOTAL = "sentry.service.client.connection.pool.max-total";
+    public static final int SENTRY_POOL_MAX_TOTAL_DEFAULT = 8;
+    public static final String SENTRY_POOL_MAX_IDLE = "sentry.service.client.connection.pool.max-idle";
+    public static final int SENTRY_POOL_MAX_IDLE_DEFAULT = 8;
+    public static final String SENTRY_POOL_MIN_IDLE = "sentry.service.client.connection.pool.min-idle";
+    public static final int SENTRY_POOL_MIN_IDLE_DEFAULT = 0;
+
+    // retry num for getting the connection from connection pool
+    public static final String SENTRY_POOL_RETRY_TOTAL = "sentry.service.client.connection.pool.retry-total";
+    public static final int SENTRY_POOL_RETRY_TOTAL_DEFAULT = 3;
+
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolHAWithoutKerberos.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolHAWithoutKerberos.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolHAWithoutKerberos.java
new file mode 100644
index 0000000..9ba7d23
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolHAWithoutKerberos.java
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless createRequired by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.thrift;
+
+import org.junit.BeforeClass;
+
+public class TestSentryServerForPoolHAWithoutKerberos extends TestSentryServerForHaWithoutKerberos {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    kerberos = false;
+    haEnabled = true;
+    pooled = true;
+    beforeSetup();
+    setupConf();
+    startSentryService();
+    afterSetup();
+  }
+
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolWithoutKerberos.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolWithoutKerberos.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolWithoutKerberos.java
new file mode 100644
index 0000000..62fbb2f
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForPoolWithoutKerberos.java
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless createRequired by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.thrift;
+
+import org.junit.BeforeClass;
+
+public class TestSentryServerForPoolWithoutKerberos extends TestSentryServerWithoutKerberos {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    kerberos = false;
+    haEnabled = false;
+    pooled = true;
+    beforeSetup();
+    setupConf();
+    startSentryService();
+    afterSetup();
+  }
+
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java
new file mode 100644
index 0000000..e5285bd
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.thrift;
+
+import static org.junit.Assert.assertTrue;
+
+import java.security.PrivilegedExceptionAction;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.FutureTask;
+
+import javax.security.auth.Subject;
+
+import org.apache.sentry.SentryUserException;
+import org.apache.sentry.service.thrift.SentryServiceFactory;
+import org.apache.sentry.service.thrift.SentryServiceIntegrationBase;
+import org.junit.Test;
+
+import com.google.common.collect.Sets;
+
+public class TestSentryServiceClientPool extends SentryServiceIntegrationBase {
+
+  @Test
+  public void testConnectionWhenReconnect() throws Exception {
+    runTestAsSubject(new TestOperation() {
+      @Override
+      public void runTestAsSubject() throws Exception {
+        String requestorUserName = ADMIN_USER;
+        Set<String> requestorUserGroupNames = Sets.newHashSet(ADMIN_GROUP);
+        String roleName = "admin_r";
+        setLocalGroupMapping(requestorUserName, requestorUserGroupNames);
+        writePolicyFile();
+
+        client.dropRoleIfExists(requestorUserName, roleName);
+        client.createRole(requestorUserName, roleName);
+        client.listRoles(requestorUserName);
+        stopSentryService();
+        server = new SentryServiceFactory().create(conf);
+        startSentryService();
+        client.listRoles(requestorUserName);
+        client.dropRole(requestorUserName, roleName);
+      }
+    });
+  }
+
+  @Test
+  public void testConnectionWithMultipleRetries() throws Exception {
+    runTestAsSubject(new TestOperation() {
+      @Override
+      public void runTestAsSubject() throws Exception {
+        List<Future<Boolean>> tasks = new ArrayList<Future<Boolean>>();
+        String requestorUserName = ADMIN_USER;
+        Set<String> requestorUserGroupNames = Sets.newHashSet(ADMIN_GROUP);
+        String roleName = "admin_r";
+        setLocalGroupMapping(requestorUserName, requestorUserGroupNames);
+        writePolicyFile();
+
+        client.dropRoleIfExists(requestorUserName, roleName);
+        client.createRole(requestorUserName, roleName);
+
+        ExecutorService executorService = Executors.newFixedThreadPool(20);
+
+        Callable<Boolean> func = new Callable<Boolean>() {
+          public Boolean call() throws Exception {
+            return Subject.doAs(clientSubject, new PrivilegedExceptionAction<Boolean>() {
+              @Override
+              public Boolean run() throws Exception {
+                try {
+                  client.listRoles(ADMIN_USER);
+                  return true;
+                } catch (SentryUserException sue) {
+                  return false;
+                }
+              }
+            });
+          }
+        };
+
+        for (int i = 0; i < 30; i++) {
+          FutureTask<Boolean> task = new FutureTask<Boolean>(func);
+          tasks.add(task);
+          executorService.submit(task);
+        }
+
+        for (Future<Boolean> task : tasks) {
+          Boolean result = task.get();
+          assertTrue("Some tasks are failed.", result);
+        }
+      }
+    });
+  }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolHAWithKerberos.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolHAWithKerberos.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolHAWithKerberos.java
new file mode 100644
index 0000000..acb906f
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolHAWithKerberos.java
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless createRequired by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.thrift;
+
+import org.junit.BeforeClass;
+
+public class TestSentryServiceForPoolHAWithKerberos extends TestSentryServiceWithKerberos {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    kerberos = true;
+    haEnabled = true;
+    pooled = true;
+    beforeSetup();
+    setupConf();
+    startSentryService();
+    afterSetup();
+  }
+
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolWithKerberos.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolWithKerberos.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolWithKerberos.java
new file mode 100644
index 0000000..bd3c1cc
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceForPoolWithKerberos.java
@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless createRequired by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.sentry.provider.db.service.thrift;
+
+import org.junit.BeforeClass;
+
+public class TestSentryServiceForPoolWithKerberos extends TestSentryServiceWithKerberos {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    kerberos = true;
+    haEnabled = false;
+    pooled = true;
+    beforeSetup();
+    setupConf();
+    startSentryService();
+    afterSetup();
+  }
+
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/7ae7fc37/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
index 9a6f8c4..1b9691e 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
@@ -100,9 +100,12 @@ public abstract class SentryServiceIntegrationBase extends SentryMiniKdcTestcase
   protected static int webServerPort = ServerConfig.SENTRY_WEB_PORT_DEFAULT;
   protected static boolean webSecurity = false;
 
+  protected static boolean pooled = false;
+
   @BeforeClass
   public static void setup() throws Exception {
     kerberos = true;
+    pooled = true;
     beforeSetup();
     setupConf();
     startSentryService();
@@ -124,6 +127,11 @@ public abstract class SentryServiceIntegrationBase extends SentryMiniKdcTestcase
     }
   }
 
+  public void stopSentryService() throws Exception {
+    server.stop();
+    Thread.sleep(30000);
+  }
+
   public static void setupConf() throws Exception {
     if (kerberos) {
       setupKdc();
@@ -179,7 +187,9 @@ public abstract class SentryServiceIntegrationBase extends SentryMiniKdcTestcase
     } else {
       conf.set(ServerConfig.SENTRY_WEB_ENABLE, "false");
     }
-
+    if (pooled) {
+      conf.set(ClientConfig.SENTRY_POOL_ENABLED, "true");
+    }
     conf.set(ServerConfig.SENTRY_VERIFY_SCHEM_VERSION, "false");
     conf.set(ServerConfig.ADMIN_GROUPS, ADMIN_GROUP);
     conf.set(ServerConfig.RPC_ADDRESS, SERVER_HOST);