sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject [2/3] sentry git commit: SENTRY-1580: Provide pooled client connection model with HA
Date Thu, 25 May 2017 04:43:41 GMT
http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java
new file mode 100644
index 0000000..e115cbb
--- /dev/null
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java
@@ -0,0 +1,39 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.core.common.transport;
+
+import com.google.common.net.HostAndPort;
+
+import java.io.IOException;
+
+/**
+ * Generic transport factory interface.
+ * <p>
+ * The intention is to implement transport pool in more abstract terms
+ * and be able to test it without actually connecting to any servers by
+ * implementing mock transport factories.
+ */
+public interface TransportFactory {
+  /**
+   * Connect to the endpoint and return a connected Thrift transport.
+   * @return Connection to the endpoint
+   * @throws IOException
+   */
+  TTransportWrapper getTransport(HostAndPort endpoint) throws IOException;
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
index d9fab86..0fffb51 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
@@ -116,12 +116,8 @@ public final class ThriftUtil {
    * (host:port). The hostname could be in ipv6 style. If port is not specified,
    * defaultPort will be used.
    */
-  public static HostAndPort[] parseHostPortStrings(String[] hostsAndPortsArr, int defaultPort) {
-    HostAndPort[] hostsAndPorts = new HostAndPort[hostsAndPortsArr.length];
-    for (int i = 0; i < hostsAndPorts.length; i++) {
-     hostsAndPorts[i] =
-          HostAndPort.fromString(hostsAndPortsArr[i]).withDefaultPort(defaultPort);
-    }
-    return hostsAndPorts;
+  public static HostAndPort parseAddress(String address, int defaultPort) {
+    return HostAndPort.fromString(address).withDefaultPort(defaultPort);
   }
+
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
index 34caa0e..7304fd8 100644
--- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
+++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
@@ -21,7 +21,7 @@ import org.apache.hadoop.conf.Configuration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class SentryUpdater {
+class SentryUpdater {
 
   private SentryHDFSServiceClient sentryClient;
   private final Configuration conf;
@@ -29,12 +29,12 @@ public class SentryUpdater {
 
   private static final Logger LOG = LoggerFactory.getLogger(SentryUpdater.class);
 
-  public SentryUpdater(Configuration conf, SentryAuthorizationInfo authzInfo) throws Exception {
+  SentryUpdater(Configuration conf, SentryAuthorizationInfo authzInfo) throws Exception {
     this.conf = conf;
     this.authzInfo = authzInfo;
   }
 
-  public SentryAuthzUpdate getUpdates() {
+  SentryAuthzUpdate getUpdates() {
     if (sentryClient == null) {
       try {
         sentryClient = SentryHDFSServiceClientFactory.create(conf);

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
index 11f6894..49d2360 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
@@ -17,19 +17,9 @@
  */
 package org.apache.sentry.hdfs;
 
-import org.apache.sentry.core.common.exception.SentryHdfsServiceException;
-
-public interface SentryHDFSServiceClient {
+public interface SentryHDFSServiceClient extends AutoCloseable {
   String SENTRY_HDFS_SERVICE_NAME = "SentryHDFSService";
 
-  void notifyHMSUpdate(PathsUpdate update)
-      throws SentryHdfsServiceException;
-
-  long getLastSeenHMSPathSeqNum() throws SentryHdfsServiceException;
-
-  SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum)
-      throws SentryHdfsServiceException;
-
-  void close();
+  SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum);
 }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
index 798bbef..1cdbb85 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
@@ -17,15 +17,12 @@
  */
 package org.apache.sentry.hdfs;
 
-import java.io.IOException;
-import java.util.LinkedList;
-
 import org.apache.hadoop.conf.Configuration;
 import org.apache.sentry.core.common.exception.SentryHdfsServiceException;
-import org.apache.sentry.core.common.transport.SentryHDFSClientTransportConfig;
-import org.apache.sentry.core.common.transport.SentryServiceClient;
-import org.apache.sentry.core.common.transport.SentryTransportFactory;
-import org.apache.sentry.hdfs.service.thrift.SentryHDFSService;
+import org.apache.sentry.core.common.transport.SentryConnection;
+import org.apache.sentry.core.common.transport.SentryTransportPool;
+import org.apache.sentry.core.common.transport.TTransportWrapper;
+import org.apache.sentry.hdfs.ServiceConstants.ClientConfig;
 import org.apache.sentry.hdfs.service.thrift.SentryHDFSService.Client;
 import org.apache.sentry.hdfs.service.thrift.TAuthzUpdateResponse;
 import org.apache.sentry.hdfs.service.thrift.TPathsUpdate;
@@ -35,82 +32,57 @@ import org.apache.thrift.protocol.TCompactProtocol;
 import org.apache.thrift.protocol.TMultiplexedProtocol;
 import org.apache.thrift.protocol.TProtocol;
 
-import org.apache.thrift.transport.TTransport;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import java.io.IOException;
+import java.util.LinkedList;
 
 /**
  * Sentry HDFS Service Client
  * <p>
- * The public implementation of SentryHDFSServiceClient.
- * A Sentry Client in which all the operations are synchronized for thread safety
- * Note: When using this client, if there is an exception in RPC, socket can get into an inconsistent state.
- * So it is important to close and re-open the transport so that new socket is used.
+ * The class isn't thread-safe - it is up to the aller to ensure thread safety
  */
-
-public class SentryHDFSServiceClientDefaultImpl implements SentryHDFSServiceClient, SentryServiceClient {
-
-  private static final Logger LOGGER = LoggerFactory.getLogger(SentryHDFSServiceClientDefaultImpl.class);
+public class SentryHDFSServiceClientDefaultImpl
+        implements SentryHDFSServiceClient, SentryConnection {
+  private final boolean useCompactTransport;
   private Client client;
-  private SentryTransportFactory transportFactory;
-  private TTransport transport;
-  private Configuration conf;
+  private final SentryTransportPool transportPool;
+  private TTransportWrapper transport;
+  private final long maxMessageSize;
 
-  public SentryHDFSServiceClientDefaultImpl(Configuration conf, SentryHDFSClientTransportConfig transportConfig) throws IOException {
-    transportFactory = new SentryTransportFactory(conf, transportConfig);
-    this.conf = conf;
+  SentryHDFSServiceClientDefaultImpl(Configuration conf,
+                                     SentryTransportPool transportPool) throws IOException {
+    maxMessageSize = conf.getLong(ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE,
+            ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
+    useCompactTransport = conf.getBoolean(ClientConfig.USE_COMPACT_TRANSPORT,
+            ClientConfig.USE_COMPACT_TRANSPORT_DEFAULT);
+    this.transportPool = transportPool;
   }
 
   /**
    * Connect to the sentry server
    *
-   * @throws IOException
+   * @throws Exception
    */
   @Override
-  public synchronized void connect() throws IOException {
-    if (transport != null && transport.isOpen()) {
+  public void connect() throws Exception {
+    if ((transport != null) && transport.isOpen()) {
       return;
     }
 
-    transport = transportFactory.getTransport();
-    TProtocol tProtocol = null;
-    long maxMessageSize = conf.getLong(ServiceConstants.ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE,
-            ServiceConstants.ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
-    if (conf.getBoolean(ServiceConstants.ClientConfig.USE_COMPACT_TRANSPORT,
-            ServiceConstants.ClientConfig.USE_COMPACT_TRANSPORT_DEFAULT)) {
-      tProtocol = new TCompactProtocol(transport, maxMessageSize, maxMessageSize);
+    transport = transportPool.getTransport();
+    TProtocol tProtocol;
+    if (useCompactTransport) {
+      tProtocol = new TCompactProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize);
     } else {
-      tProtocol = new TBinaryProtocol(transport, maxMessageSize, maxMessageSize, true, true);
+      tProtocol = new TBinaryProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize, true, true);
     }
     TMultiplexedProtocol protocol = new TMultiplexedProtocol(
             tProtocol, SentryHDFSServiceClient.SENTRY_HDFS_SERVICE_NAME);
 
-    client = new SentryHDFSService.Client(protocol);
-    LOGGER.info("Successfully created client");
-  }
-
-  @Override
-  public synchronized void notifyHMSUpdate(PathsUpdate update)
-          throws SentryHdfsServiceException {
-    try {
-      client.handle_hms_notification(update.toThrift());
-    } catch (Exception e) {
-      throw new SentryHdfsServiceException("Thrift Exception occurred !!", e);
-    }
-  }
-
-  @Override
-  public synchronized long getLastSeenHMSPathSeqNum()
-          throws SentryHdfsServiceException {
-    try {
-      return client.check_hms_seq_num(-1);
-    } catch (Exception e) {
-      throw new SentryHdfsServiceException("Thrift Exception occurred !!", e);
-    }
+    client = new Client(protocol);
   }
 
   @Override
-  public synchronized SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum)
+  public SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum)
           throws SentryHdfsServiceException {
     SentryAuthzUpdate retVal = new SentryAuthzUpdate(new LinkedList<PermissionsUpdate>(), new LinkedList<PathsUpdate>());
     try {
@@ -132,12 +104,23 @@ public class SentryHDFSServiceClientDefaultImpl implements SentryHDFSServiceClie
   }
 
   @Override
-  public synchronized void close() {
-    transportFactory.close();
+  public void close() {
+    done();
   }
 
   @Override
-  public void disconnect() {
-    transportFactory.releaseTransport();
+  public void done() {
+    if (transport != null) {
+      transportPool.returnTransport(transport);
+      transport = null;
+    }
+  }
+
+  @Override
+  public void invalidate() {
+    if (transport != null) {
+      transportPool.invalidateTransport(transport);
+      transport = null;
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
index e350103..fb34b0b 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
@@ -18,28 +18,107 @@
 package org.apache.sentry.hdfs;
 
 import java.lang.reflect.Proxy;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.sentry.core.common.transport.RetryClientInvocationHandler;
 import org.apache.sentry.core.common.transport.SentryHDFSClientTransportConfig;
+import org.apache.sentry.core.common.transport.SentryTransportFactory;
+import org.apache.sentry.core.common.transport.SentryTransportPool;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.annotation.concurrent.ThreadSafe;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static org.apache.sentry.core.common.utils.SentryConstants.KERBEROS_MODE;
 
 /**
- * Client factory to create normal client or proxy with HA invocation handler
+ * Client factory for creating HDFS service clients.
+ * This is a singleton which uses a single factory.
  */
+@ThreadSafe
 public class SentryHDFSServiceClientFactory {
-  private final static SentryHDFSClientTransportConfig transportConfig =
+  private static final Logger LOGGER = LoggerFactory.getLogger(SentryHDFSServiceClientFactory.class);
+
+  private static final AtomicReference<SentryHDFSServiceClientFactory> clientFactory =
+          new AtomicReference<>();
+
+  private final SentryHDFSClientTransportConfig transportConfig =
           new SentryHDFSClientTransportConfig();
+  private final Configuration conf;
+  private final SentryTransportPool transportPool;
+
+  /**
+   * Return a client instance
+   * @param conf
+   * @return
+   * @throws Exception
+   */
+  public static SentryHDFSServiceClient create(Configuration conf) throws Exception {
+    SentryHDFSServiceClientFactory factory = clientFactory.get();
+    if (factory != null) {
+      return factory.create();
+    }
+    factory = new SentryHDFSServiceClientFactory(conf);
+    boolean ok = clientFactory.compareAndSet(null, factory);
+    if (ok) {
+      return factory.create();
+    }
+    factory.close();
+    return clientFactory.get().create();
+  }
+
+  private SentryHDFSServiceClientFactory(Configuration conf) {
+    Configuration clientConf = conf;
 
-  private SentryHDFSServiceClientFactory() {
-    // Make constructor private to avoid instantiation
+    // When kerberos is enabled,  UserGroupInformation should have been initialized with
+    // HADOOP_SECURITY_AUTHENTICATION property. There are instances where this is not done.
+    // Instead of depending on the callers to update this configuration and to be
+    // sure that UserGroupInformation is properly initialized, sentry client is explicitly
+    // doing it.
+    //
+    // This whole piece of code is a bit ugly but we want to avoid doing this in the transport
+    // code during connection establishment, so we are doing it upfront here instead.
+    boolean useKerberos = transportConfig.isKerberosEnabled(conf);
+
+    if (useKerberos) {
+      LOGGER.info("Using Kerberos authentication");
+      String authMode = conf.get(HADOOP_SECURITY_AUTHENTICATION, "");
+      if (authMode != KERBEROS_MODE) {
+        // Force auth mode to be Kerberos
+        clientConf = new Configuration(conf);
+        clientConf.set(HADOOP_SECURITY_AUTHENTICATION, KERBEROS_MODE);
+      }
+    }
+
+    this.conf = clientConf;
+
+    boolean useUGI = transportConfig.useUserGroupInformation(conf);
+
+    if (useUGI) {
+      LOGGER.info("Using UserGroupInformation authentication");
+      UserGroupInformation.setConfiguration(this.conf);
+    }
+
+    transportPool = new SentryTransportPool(conf, transportConfig,
+            new SentryTransportFactory(conf, transportConfig));
   }
 
-  public static SentryHDFSServiceClient create(Configuration conf)
-    throws Exception {
+  private SentryHDFSServiceClient create() throws Exception {
     return (SentryHDFSServiceClient) Proxy
       .newProxyInstance(SentryHDFSServiceClientDefaultImpl.class.getClassLoader(),
         SentryHDFSServiceClientDefaultImpl.class.getInterfaces(),
         new RetryClientInvocationHandler(conf,
-          new SentryHDFSServiceClientDefaultImpl(conf, transportConfig), transportConfig));
+          new SentryHDFSServiceClientDefaultImpl(conf, transportPool), transportConfig));
+  }
+
+  void close() {
+    try {
+      transportPool.close();
+    } catch (Exception e) {
+      LOGGER.error("failed to close transport pool", e);
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
index eccf83b..3ee3724 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
@@ -44,7 +44,10 @@ public class SentryHdfsServiceIntegrationBase extends
   @After
   public void after() {
     if (hdfsClient != null) {
-      hdfsClient.close();
+      try {
+        hdfsClient.close();
+      } catch (Exception ignored) {
+      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java
index 75d2993..480991d 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java
@@ -77,9 +77,8 @@ public class SimpleDBProviderBackend implements ProviderBackend {
     int retries = Math.max(retryCount + 1, 1); // if customer configs retryCount as Integer.MAX_VALUE, try only once
     while (retries > 0) {
       retries--;
-      SentryPolicyServiceClient policyServiceClient = null;
-      try {
-        policyServiceClient = SentryServiceClientFactory.create(conf);
+      try (SentryPolicyServiceClient policyServiceClient =
+                   SentryServiceClientFactory.create(conf)) {
         return ImmutableSet.copyOf(policyServiceClient.listPrivilegesForProvider(groups, users,
             roleSet, authorizableHierarchy));
       } catch (Exception e) {
@@ -97,10 +96,6 @@ public class SimpleDBProviderBackend implements ProviderBackend {
             LOGGER.info("Sleeping is interrupted.", e1);
           }
         }
-      } finally {
-        if(policyServiceClient != null) {
-          policyServiceClient.close();
-        }
       }
     }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
index 134012d..6c7d3ef 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java
@@ -82,6 +82,7 @@ public class SentryGenericProviderBackend extends CacheProvider implements Provi
       } catch (NoSuchMethodException | ClassNotFoundException | InstantiationException | InvocationTargetException | IllegalAccessException e) {
         throw new RuntimeException("Failed to create privilege converter of type " + privilegeConverter, e);
       }
+      LOGGER.debug("Starting Updateable Cache");
       UpdatableCache cache = new UpdatableCache(conf, getComponentType(), getServiceName(), sentryPrivilegeConverter);
       try {
         cache.startUpdateThread(true);
@@ -110,9 +111,7 @@ public class SentryGenericProviderBackend extends CacheProvider implements Provi
     if (enableCaching) {
       return super.getPrivileges(groups, roleSet, authorizableHierarchy);
     } else {
-      SentryGenericServiceClient client = null;
-      try {
-        client = getClient();
+      try (SentryGenericServiceClient client = getClient()){
         return ImmutableSet.copyOf(client.listPrivilegesForProvider(componentType, serviceName,
             roleSet, groups, Arrays.asList(authorizableHierarchy)));
       } catch (SentryUserException e) {
@@ -121,10 +120,6 @@ public class SentryGenericProviderBackend extends CacheProvider implements Provi
       } catch (Exception e) {
         String msg = "Unable to obtain client:" + e.getMessage();
         LOGGER.error(msg, e);
-      } finally {
-        if (client != null) {
-          client.close();
-        }
       }
     }
     return ImmutableSet.of();
@@ -138,10 +133,8 @@ public class SentryGenericProviderBackend extends CacheProvider implements Provi
     if (enableCaching) {
       return super.getRoles(groups, roleSet);
     } else {
-      SentryGenericServiceClient client = null;
-      try {
+      try (SentryGenericServiceClient client = getClient()){
         Set<TSentryRole> tRoles = Sets.newHashSet();
-        client = getClient();
         //get the roles according to group
         String requestor = UserGroupInformation.getCurrentUser().getShortUserName();
         for (String group : groups) {
@@ -158,10 +151,6 @@ public class SentryGenericProviderBackend extends CacheProvider implements Provi
       } catch (Exception e) {
         String msg = "Unable to obtain client:" + e.getMessage();
         LOGGER.error(msg, e);
-      } finally {
-        if (client != null) {
-          client.close();
-        }
       }
       return ImmutableSet.of();
     }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
index 41708c3..d20710f 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
@@ -29,9 +29,15 @@ import java.util.Timer;
 import java.util.TimerTask;
 import java.util.concurrent.TimeUnit;
 
-class UpdatableCache implements TableCache {
+public final class UpdatableCache implements TableCache, AutoCloseable {
   private static final Logger LOGGER = LoggerFactory.getLogger(UpdatableCache.class);
 
+  // Timer for getting updates periodically
+  private final Timer timer = new Timer();
+  private boolean initialized = false;
+  // saved timer is used by tests to cancel previous timer
+  private static Timer savedTimer;
+
   private final String componentType;
   private final String serviceName;
   private final long cacheTtlNs;
@@ -94,14 +100,13 @@ class UpdatableCache implements TableCache {
     String requestor;
     requestor = UserGroupInformation.getLoginUser().getShortUserName();
 
-    SentryGenericServiceClient client = null;
-    try {
-      client = getClient();
-      final Set<TSentryRole> tSentryRoles = client.listAllRoles(requestor, componentType);
+    try(SentryGenericServiceClient client = getClient()) {
+      Set<TSentryRole>  tSentryRoles = client.listAllRoles(requestor, componentType);
 
       for (TSentryRole tSentryRole : tSentryRoles) {
         final String roleName = tSentryRole.getRoleName();
-        final Set<TSentryPrivilege> tSentryPrivileges = client.listPrivilegesByRoleName(requestor, roleName, componentType, serviceName);
+        final Set<TSentryPrivilege> tSentryPrivileges =
+                client.listPrivilegesByRoleName(requestor, roleName, componentType, serviceName);
         for (String group : tSentryRole.getGroups()) {
           Set<String> currentPrivileges = tempCache.get(group, roleName);
           if (currentPrivileges == null) {
@@ -113,12 +118,8 @@ class UpdatableCache implements TableCache {
           }
         }
       }
-    } finally {
-      if (client != null) {
-        client.close();
-      }
+      return tempCache;
     }
-    return tempCache;
   }
 
   /**
@@ -136,7 +137,19 @@ class UpdatableCache implements TableCache {
       reloadData();
     }
 
-    Timer timer = new Timer();
+    if (initialized) {
+      LOGGER.info("Already initialized");
+      return;
+    }
+
+    initialized = true;
+    // Save timer to be able to cancel it.
+    if (savedTimer != null) {
+      LOGGER.debug("Resetting saved timer");
+      savedTimer.cancel();
+    }
+    savedTimer = timer;
+
     final long refreshIntervalMs = TimeUnit.NANOSECONDS.toMillis(cacheTtlNs);
     timer.scheduleAtFixedRate(
         new TimerTask() {
@@ -158,6 +171,7 @@ class UpdatableCache implements TableCache {
 
   private void revokeAllPrivilegesIfRequired() {
     if (++consecutiveUpdateFailuresCount > allowedUpdateFailuresCount) {
+      consecutiveUpdateFailuresCount = 0;
       // Clear cache to revoke all privileges.
       // Update table cache to point to an empty table to avoid thread-unsafe characteristics of HashBasedTable.
       this.table = HashBasedTable.create();
@@ -175,4 +189,21 @@ class UpdatableCache implements TableCache {
     final long currentTimeNs = System.nanoTime();
     return lastRefreshedNs + cacheTtlNs < currentTimeNs;
   }
+
+  /**
+   * Only called by tests to disable timer.
+   */
+  public static void disable() {
+    if (savedTimer != null) {
+      LOGGER.info("Disabling timer");
+      savedTimer.cancel();
+    }
+  }
+
+  @Override
+  public void close() {
+    timer.cancel();
+    savedTimer = null;
+    LOGGER.info("Closed Updatable Cache");
+  }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java
index 11cdee7..246d0b4 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java
@@ -25,7 +25,7 @@ import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
 
-public interface SentryGenericServiceClient {
+public interface SentryGenericServiceClient extends AutoCloseable {
 
   /**
    * Create a sentry role
@@ -191,6 +191,4 @@ public interface SentryGenericServiceClient {
   Map<String, TSentryPrivilegeMap> listPrivilegsbyAuthorizable(String component,
       String serviceName, String requestorUserName, Set<String> authorizablesSet,
       Set<String> groups, ActiveRoleSet roleSet) throws SentryUserException;
-
-  void close();
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
index e23d13b..6301a6b 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
@@ -17,52 +17,63 @@
  */
 package org.apache.sentry.provider.db.generic.service.thrift;
 
-import java.io.IOException;
-import java.util.*;
-
+import com.google.common.collect.Lists;
 import org.apache.hadoop.conf.Configuration;
-
-import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
-import org.apache.sentry.core.common.transport.SentryPolicyClientTransportConfig;
-import org.apache.sentry.core.common.transport.SentryServiceClient;
-import org.apache.sentry.core.common.transport.SentryTransportFactory;
+import org.apache.sentry.core.common.exception.SentryUserException;
+import org.apache.sentry.core.common.transport.SentryConnection;
+import org.apache.sentry.core.common.transport.SentryTransportPool;
+import org.apache.sentry.core.common.transport.TTransportWrapper;
 import org.apache.sentry.core.model.db.AccessConstants;
-import org.apache.sentry.service.thrift.ServiceConstants;
+import org.apache.sentry.provider.db.generic.service.thrift.SentryGenericPolicyService.Client;
+import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
 import org.apache.sentry.service.thrift.Status;
 import org.apache.sentry.service.thrift.sentry_common_serviceConstants;
 import org.apache.thrift.TException;
 import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TMultiplexedProtocol;
 
-import org.apache.thrift.transport.TTransport;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
-import com.google.common.collect.Lists;
 
 /**
- * Sentry Generic Service Client
+ * Sentry Generic Service Client.
  * <p>
- * The public implementation of SentryGenericServiceClient.
- * TODO(kalyan) A Sentry Client in which all the operations are synchronized for thread safety
- * Note: When using this client, if there is an exception in RPC, socket can get into an inconsistent state.
- * So it is important to close and re-open the transportFactory so that new socket is used.
+ * Thread safety. This class is not thread safe - it is up to the
+ * caller to ensure thread safety.
  */
+public class SentryGenericServiceClientDefaultImpl
+        implements SentryGenericServiceClient, SentryConnection {
 
-public class SentryGenericServiceClientDefaultImpl implements SentryGenericServiceClient, SentryServiceClient {
-  private SentryGenericPolicyService.Client client;
-  private SentryTransportFactory transportFactory;
-  private TTransport transport;
-  private Configuration conf;
-  private static final Logger LOGGER = LoggerFactory
-    .getLogger(SentryGenericServiceClientDefaultImpl.class);
+  private Client client;
+  private final SentryTransportPool transportPool;
+  private TTransportWrapper transport;
   private static final String THRIFT_EXCEPTION_MESSAGE = "Thrift exception occured ";
+  private final long maxMessageSize;
 
-  public SentryGenericServiceClientDefaultImpl(Configuration conf, SentryPolicyClientTransportConfig transportConfig) throws IOException {
-    transportFactory = new SentryTransportFactory(conf, transportConfig);
-    this.conf = conf;
+  /**
+   * Initialize client with the given configuration, using specified transport pool
+   * implementation for obtaining transports.
+   * @param conf Sentry Configuration
+   * @param transportPool source of connected transports
+   */
+  SentryGenericServiceClientDefaultImpl(Configuration conf,
+                                        SentryTransportPool transportPool) {
+
+    //TODO(kalyan) need to find appropriate place to add it
+    // if (kerberos) {
+    //  // since the client uses hadoop-auth, we need to set kerberos in
+    //  // hadoop-auth if we plan to use kerberos
+    //  conf.set(HADOOP_SECURITY_AUTHENTICATION, SentryConstants.KERBEROS_MoODE);
+    // }
+    maxMessageSize = conf.getLong(ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE,
+            ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
+    this.transportPool = transportPool;
   }
 
   /**
@@ -71,20 +82,18 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi
    * @throws IOException
    */
   @Override
-  public synchronized void connect() throws IOException {
-    if (transport != null && transport.isOpen()) {
+  public void connect() throws Exception {
+    if ((transport != null) && transport.isOpen()) {
       return;
     }
 
-    transport = transportFactory.getTransport();
-    TMultiplexedProtocol protocol = null;
-    long maxMessageSize = conf.getLong(ServiceConstants.ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE,
-      ServiceConstants.ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
-    protocol = new TMultiplexedProtocol(
-      new TBinaryProtocol(transport, maxMessageSize, maxMessageSize, true, true),
+    // Obtain connection to Sentry server
+    transport = transportPool.getTransport();
+    TMultiplexedProtocol protocol = new TMultiplexedProtocol(
+      new TBinaryProtocol(transport.getTTransport(), maxMessageSize,
+              maxMessageSize, true, true),
       SentryGenericPolicyProcessor.SENTRY_GENERIC_SERVICE_NAME);
-    client = new SentryGenericPolicyService.Client(protocol);
-    LOGGER.debug("Successfully created client");
+    client = new Client(protocol);
   }
 
   /**
@@ -96,7 +105,7 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi
    * @throws SentryUserException
    */
   @Override
-  public synchronized void createRole(String requestorUserName, String roleName, String component)
+  public void createRole(String requestorUserName, String roleName, String component)
     throws SentryUserException {
     TCreateSentryRoleRequest request = new TCreateSentryRoleRequest();
     request.setProtocol_version(sentry_common_serviceConstants.TSENTRY_SERVICE_V2);
@@ -359,7 +368,7 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi
    * @throws SentryUserException
    */
   @Override
-  public synchronized Set<TSentryRole> listRolesByGroupName(
+  public Set<TSentryRole> listRolesByGroupName(
     String requestorUserName,
     String groupName,
     String component)
@@ -528,12 +537,23 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi
   }
 
   @Override
-  public synchronized void close() {
-    transportFactory.close();
+  public void close() {
+    done();
   }
 
   @Override
-  public void disconnect() {
-    transportFactory.releaseTransport();
+  public void done() {
+    if (transport != null) {
+      transportPool.returnTransport(transport);
+      transport = null;
+    }
+  }
+
+  @Override
+  public void invalidate() {
+    if (transport != null) {
+      transportPool.invalidateTransport(transport);
+      transport = null;
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
index 46ac4a3..1b47236 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
@@ -18,27 +18,131 @@
 package org.apache.sentry.provider.db.generic.service.thrift;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.sentry.core.common.transport.RetryClientInvocationHandler;
 import org.apache.sentry.core.common.transport.SentryPolicyClientTransportConfig;
+import org.apache.sentry.core.common.transport.SentryTransportFactory;
+import org.apache.sentry.core.common.transport.SentryTransportPool;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import javax.annotation.concurrent.ThreadSafe;
 import java.lang.reflect.Proxy;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static org.apache.sentry.core.common.utils.SentryConstants.KERBEROS_MODE;
 
 /**
- * SentryGenericServiceClientFactory is a public class for the components which using Generic Model to create sentry client.
+ * Produces client connection for Sentry clients using Generic model.
+ * Factory is [alost] a singleton. Tests can call {@link #factoryReset()} to destroy the
+ * existing factory and create a new one. This may be needed because tests modify
+ * configuration and start and stop servers.
  */
+@ThreadSafe
 public final class SentryGenericServiceClientFactory {
-  private static final SentryPolicyClientTransportConfig transportConfig =
+  private static final Logger LOGGER = LoggerFactory.getLogger(SentryGenericServiceClientFactory.class);
+
+  // Used to implement a singleton
+  private static final AtomicReference<SentryGenericServiceClientFactory> clientFactory =
+          new AtomicReference<>();
+
+  private final SentryPolicyClientTransportConfig transportConfig =
           new SentryPolicyClientTransportConfig();
+  private final SentryTransportPool transportPool;
+  private final Configuration conf;
 
-  private SentryGenericServiceClientFactory() {
+  /**
+   * Obtain an Generic policy client instance.
+   * @param conf Configuration that should be used. Configuration is only used for the
+   *             initial creation and ignored afterwords.
+   */
+  public static SentryGenericServiceClient create(Configuration conf) throws Exception {
+    SentryGenericServiceClientFactory factory = clientFactory.get();
+    if (factory != null) {
+      return factory.create();
+    }
+    factory = new SentryGenericServiceClientFactory(conf);
+    boolean ok = clientFactory.compareAndSet(null, factory);
+    if (ok) {
+      return factory.create();
+    }
+    factory.close();
+    return clientFactory.get().create();
   }
 
-  public static SentryGenericServiceClient create(Configuration conf) throws Exception {
+  /**
+   * Create a new factory instance and atach it to a connection pool instance.
+   * @param conf Configuration
+   */
+  private SentryGenericServiceClientFactory(Configuration conf) {
+    Configuration clientConf = conf;
+
+    // When kerberos is enabled,  UserGroupInformation should have been initialized with
+    // HADOOP_SECURITY_AUTHENTICATION property. There are instances where this is not done.
+    // Instead of depending on the callers to update this configuration and to be
+    // sure that UserGroupInformation is properly initialized, sentry client is explicitly
+    // doing it.
+    //
+    // This whole piece of code is a bit ugly but we want to avoid doing this in the transport
+    // code during connection establishment, so we are doing it upfront here instead.
+    boolean useKerberos = transportConfig.isKerberosEnabled(conf);
+
+    if (useKerberos) {
+      LOGGER.info("Using Kerberos authentication");
+      String authMode = conf.get(HADOOP_SECURITY_AUTHENTICATION, "");
+      if (authMode != KERBEROS_MODE) {
+        // Force auth mode to be Kerberos
+        clientConf = new Configuration(conf);
+        clientConf.set(HADOOP_SECURITY_AUTHENTICATION, KERBEROS_MODE);
+      }
+    }
+
+    this.conf = clientConf;
+
+    boolean useUGI = transportConfig.useUserGroupInformation(conf);
+
+    if (useUGI) {
+      LOGGER.info("Using UserGroupInformation authentication");
+      UserGroupInformation.setConfiguration(this.conf);
+    }
+
+    transportPool = new SentryTransportPool(conf, transportConfig,
+            new SentryTransportFactory(conf, transportConfig));
+  }
+
+  /**
+   * Create a new client connection to the server for Generic model clients
+   * @return client instance
+   * @throws Exception if something goes wrong
+   */
+  private SentryGenericServiceClient create() throws Exception {
     return (SentryGenericServiceClient) Proxy
       .newProxyInstance(SentryGenericServiceClientDefaultImpl.class.getClassLoader(),
         SentryGenericServiceClientDefaultImpl.class.getInterfaces(),
         new RetryClientInvocationHandler(conf,
-          new SentryGenericServiceClientDefaultImpl(conf, transportConfig), transportConfig));
+          new SentryGenericServiceClientDefaultImpl(conf, transportPool), transportConfig));
+  }
+
+  // Should only be used by tests.
+  // Resets the factory and destroys any pooled connections
+  public static void factoryReset() {
+    LOGGER.debug("factory reset");
+    SentryGenericServiceClientFactory factory = clientFactory.getAndSet(null);
+    if (factory != null) {
+      try {
+        factory.transportPool.close();
+      } catch (Exception e) {
+        LOGGER.error("failed to close transport pool", e);
+      }
+    }
   }
 
+  void close() {
+    try {
+      transportPool.close();
+    } catch (Exception e) {
+      LOGGER.error("failed to close transport pool", e);
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
index 404adb8..b958b09 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java
@@ -62,12 +62,14 @@ public class SentryConfigToolSolr extends SentryConfigToolCommon {
     String service = conf.get(SOLR_SERVICE_NAME, "service1");
     // instantiate a solr client for sentry service.  This sets the ugi, so must
     // be done before getting the ugi below.
-    SentryGenericServiceClient client = SentryGenericServiceClientFactory.create(conf);
-    UserGroupInformation ugi = UserGroupInformation.getLoginUser();
-    String requestorName = ugi.getShortUserName();
+    try(SentryGenericServiceClient client =
+                SentryGenericServiceClientFactory.create(conf)) {
+      UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+      String requestorName = ugi.getShortUserName();
 
-    convertINIToSentryServiceCmds(component, service, requestorName, conf, client,
-        getPolicyFile(), getValidate(), getImportPolicy(), getCheckCompat());
+      convertINIToSentryServiceCmds(component, service, requestorName, conf, client,
+              getPolicyFile(), getValidate(), getImportPolicy(), getCheckCompat());
+    }
   }
 
   private Configuration getSentryConf() {

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java
index d6d9014..f6e5d1b 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java
@@ -48,39 +48,41 @@ public class SentryShellKafka extends SentryShellCommon {
     Configuration conf = getSentryConf();
 
     String service = conf.get(KAFKA_SERVICE_NAME, "kafka1");
-    SentryGenericServiceClient client = SentryGenericServiceClientFactory.create(conf);
-    UserGroupInformation ugi = UserGroupInformation.getLoginUser();
-    String requestorName = ugi.getShortUserName();
+    try(SentryGenericServiceClient client =
+                SentryGenericServiceClientFactory.create(conf)) {
+      UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+      String requestorName = ugi.getShortUserName();
 
-    if (isCreateRole) {
-      command = new CreateRoleCmd(roleName, component);
-    } else if (isDropRole) {
-      command = new DropRoleCmd(roleName, component);
-    } else if (isAddRoleGroup) {
-      command = new AddRoleToGroupCmd(roleName, groupName, component);
-    } else if (isDeleteRoleGroup) {
-      command = new DeleteRoleFromGroupCmd(roleName, groupName, component);
-    } else if (isGrantPrivilegeRole) {
-      command = new GrantPrivilegeToRoleCmd(roleName, component,
-          privilegeStr, new KafkaTSentryPrivilegeConverter(component, service));
-    } else if (isRevokePrivilegeRole) {
-      command = new RevokePrivilegeFromRoleCmd(roleName, component,
-          privilegeStr, new KafkaTSentryPrivilegeConverter(component, service));
-    } else if (isListRole) {
-      command = new ListRolesCmd(groupName, component);
-    } else if (isListPrivilege) {
-      command = new ListPrivilegesByRoleCmd(roleName, component,
-          service, new KafkaTSentryPrivilegeConverter(component, service));
-    }
+      if (isCreateRole) {
+        command = new CreateRoleCmd(roleName, component);
+      } else if (isDropRole) {
+        command = new DropRoleCmd(roleName, component);
+      } else if (isAddRoleGroup) {
+        command = new AddRoleToGroupCmd(roleName, groupName, component);
+      } else if (isDeleteRoleGroup) {
+        command = new DeleteRoleFromGroupCmd(roleName, groupName, component);
+      } else if (isGrantPrivilegeRole) {
+        command = new GrantPrivilegeToRoleCmd(roleName, component,
+                privilegeStr, new KafkaTSentryPrivilegeConverter(component, service));
+      } else if (isRevokePrivilegeRole) {
+        command = new RevokePrivilegeFromRoleCmd(roleName, component,
+                privilegeStr, new KafkaTSentryPrivilegeConverter(component, service));
+      } else if (isListRole) {
+        command = new ListRolesCmd(groupName, component);
+      } else if (isListPrivilege) {
+        command = new ListPrivilegesByRoleCmd(roleName, component,
+                service, new KafkaTSentryPrivilegeConverter(component, service));
+      }
 
-    // check the requestor name
-    if (StringUtils.isEmpty(requestorName)) {
-      // The exception message will be recorded in log file.
-      throw new Exception("The requestor name is empty.");
-    }
+      // check the requestor name
+      if (StringUtils.isEmpty(requestorName)) {
+        // The exception message will be recorded in log file.
+        throw new Exception("The requestor name is empty.");
+      }
 
-    if (command != null) {
-      command.execute(client, requestorName);
+      if (command != null) {
+        command.execute(client, requestorName);
+      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java
index 695c008..5385f7d 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java
@@ -47,39 +47,41 @@ public class SentryShellSolr extends SentryShellCommon {
     Configuration conf = getSentryConf();
 
     String service = conf.get(SOLR_SERVICE_NAME, "service1");
-    SentryGenericServiceClient client = SentryGenericServiceClientFactory.create(conf);
-    UserGroupInformation ugi = UserGroupInformation.getLoginUser();
-    String requestorName = ugi.getShortUserName();
+    try(SentryGenericServiceClient client =
+                SentryGenericServiceClientFactory.create(conf)) {
+      UserGroupInformation ugi = UserGroupInformation.getLoginUser();
+      String requestorName = ugi.getShortUserName();
 
-    if (isCreateRole) {
-      command = new CreateRoleCmd(roleName, component);
-    } else if (isDropRole) {
-      command = new DropRoleCmd(roleName, component);
-    } else if (isAddRoleGroup) {
-      command = new AddRoleToGroupCmd(roleName, groupName, component);
-    } else if (isDeleteRoleGroup) {
-      command = new DeleteRoleFromGroupCmd(roleName, groupName, component);
-    } else if (isGrantPrivilegeRole) {
-      command = new GrantPrivilegeToRoleCmd(roleName, component,
-          privilegeStr, new SolrTSentryPrivilegeConverter(component, service));
-    } else if (isRevokePrivilegeRole) {
-      command = new RevokePrivilegeFromRoleCmd(roleName, component,
-          privilegeStr, new SolrTSentryPrivilegeConverter(component, service));
-    } else if (isListRole) {
-      command = new ListRolesCmd(groupName, component);
-    } else if (isListPrivilege) {
-      command = new ListPrivilegesByRoleCmd(roleName, component,
-          service, new SolrTSentryPrivilegeConverter(component, service));
-    }
+      if (isCreateRole) {
+        command = new CreateRoleCmd(roleName, component);
+      } else if (isDropRole) {
+        command = new DropRoleCmd(roleName, component);
+      } else if (isAddRoleGroup) {
+        command = new AddRoleToGroupCmd(roleName, groupName, component);
+      } else if (isDeleteRoleGroup) {
+        command = new DeleteRoleFromGroupCmd(roleName, groupName, component);
+      } else if (isGrantPrivilegeRole) {
+        command = new GrantPrivilegeToRoleCmd(roleName, component,
+                privilegeStr, new SolrTSentryPrivilegeConverter(component, service));
+      } else if (isRevokePrivilegeRole) {
+        command = new RevokePrivilegeFromRoleCmd(roleName, component,
+                privilegeStr, new SolrTSentryPrivilegeConverter(component, service));
+      } else if (isListRole) {
+        command = new ListRolesCmd(groupName, component);
+      } else if (isListPrivilege) {
+        command = new ListPrivilegesByRoleCmd(roleName, component,
+                service, new SolrTSentryPrivilegeConverter(component, service));
+      }
 
-    // check the requestor name
-    if (StringUtils.isEmpty(requestorName)) {
-      // The exception message will be recorded in log file.
-      throw new Exception("The requestor name is empty.");
-    }
+      // check the requestor name
+      if (StringUtils.isEmpty(requestorName)) {
+        // The exception message will be recorded in log file.
+        throw new Exception("The requestor name is empty.");
+      }
 
-    if (command != null) {
-      command.execute(client, requestorName);
+      if (command != null) {
+        command.execute(client, requestorName);
+      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
index c2b03e5..fb8036f 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
@@ -26,7 +26,7 @@ import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
 
-public interface SentryPolicyServiceClient {
+public interface SentryPolicyServiceClient extends AutoCloseable {
 
   void createRole(String requestorUserName, String roleName) throws SentryUserException;
 
@@ -208,8 +208,6 @@ public interface SentryPolicyServiceClient {
    */
   String getConfigValue(String propertyName, String defaultValue) throws SentryUserException;
 
-  void close();
-
   // Import the sentry mapping data with map structure
   void importPolicy(Map<String, Map<String, Set<String>>> policyFileMappingData,
       String requestorUserName, boolean isOverwriteRole) throws SentryUserException;

http://git-wip-us.apache.org/repos/asf/sentry/blob/95d073f0/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
index d1a4d99..b5b8f82 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
@@ -18,78 +18,62 @@
 
 package org.apache.sentry.provider.db.service.thrift;
 
-import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.Collections;
-
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
-
-import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
-import org.apache.sentry.core.common.transport.SentryTransportFactory;
+import org.apache.sentry.core.common.exception.SentryUserException;
+import org.apache.sentry.core.common.transport.SentryConnection;
+import org.apache.sentry.core.common.transport.SentryTransportPool;
+import org.apache.sentry.core.common.transport.TTransportWrapper;
+import org.apache.sentry.core.common.utils.PolicyFileConstants;
 import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.core.model.db.DBModelAuthorizable;
-import org.apache.sentry.core.common.utils.PolicyFileConstants;
+import org.apache.sentry.provider.db.service.thrift.SentryPolicyService.Client;
 import org.apache.sentry.service.thrift.SentryServiceUtil;
-import org.apache.sentry.service.thrift.ServiceConstants;
+import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
 import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope;
 import org.apache.sentry.service.thrift.ServiceConstants.ThriftConstants;
 import org.apache.sentry.service.thrift.Status;
 import org.apache.thrift.TException;
 import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TMultiplexedProtocol;
-import org.apache.sentry.core.common.transport.SentryServiceClient;
-import org.apache.sentry.core.common.transport.SentryPolicyClientTransportConfig;
-import org.apache.thrift.transport.TTransport;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 /**
- * Sentry Policy Service Client
+ * Client implementation for Policy (HMS) clients.
  * <p>
- * The public implementation of SentryPolicyServiceClient.
- * Note: When using this client, if there is an exception in RPC, socket can get into an inconsistent state
- * So it is important to close and re-open the transportFactory so that new socket is used.
- * When an class is instantiated, there will be transportFactory created connecting with first available
- * server this is configured.
+ * The class is not thread-safe - it is up to the callers to ensure thread safety
  */
-public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyServiceClient, SentryServiceClient {
+public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyServiceClient, SentryConnection {
 
-  private SentryPolicyService.Client client;
-  private SentryTransportFactory transportFactory;
-  private TTransport transport;
-  private Configuration conf;
+  private Client client;
+  private final SentryTransportPool transportPool;
+  private TTransportWrapper transport;
+  private final long maxMessageSize;
 
-  private static final Logger LOGGER = LoggerFactory
-    .getLogger(SentryPolicyServiceClient.class);
   private static final String THRIFT_EXCEPTION_MESSAGE = "Thrift exception occurred ";
 
   /**
    * Initialize the sentry configurations.
    */
-  public SentryPolicyServiceClientDefaultImpl(Configuration conf, SentryPolicyClientTransportConfig transportConfig)
+  public SentryPolicyServiceClientDefaultImpl(Configuration conf,
+                                              SentryTransportPool transportPool)
     throws IOException {
-    transportFactory = new SentryTransportFactory(conf, transportConfig);
-    this.conf = conf;
-  }
-
-  public SentryPolicyServiceClientDefaultImpl(String addr, int port,
-                                              Configuration conf) throws IOException {
-    transportFactory = new SentryTransportFactory(addr, port, conf,
-      new SentryPolicyClientTransportConfig());
-    this.conf = conf;
-    connect();
+    maxMessageSize = conf.getLong(ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE,
+            ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
+    this.transportPool = transportPool;
   }
 
   /**
@@ -98,24 +82,21 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    * @throws IOException
    */
   @Override
-  public synchronized void connect() throws IOException {
-    if (transport != null && transport.isOpen()) {
+  public void connect() throws Exception {
+    if ((transport != null) && transport.isOpen()) {
       return;
     }
 
-    transport = transportFactory.getTransport();
-    long maxMessageSize = conf.getLong(
-      ServiceConstants.ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE,
-      ServiceConstants.ClientConfig.SENTRY_POLICY_CLIENT_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
+    transport = transportPool.getTransport();
     TMultiplexedProtocol protocol = new TMultiplexedProtocol(
-      new TBinaryProtocol(transport, maxMessageSize, maxMessageSize, true, true),
-      SentryPolicyStoreProcessor.SENTRY_POLICY_SERVICE_NAME);
-    client = new SentryPolicyService.Client(protocol);
-    LOGGER.debug("Successfully created client");
+      new TBinaryProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize,
+              true, true),
+            SentryPolicyStoreProcessor.SENTRY_POLICY_SERVICE_NAME);
+    client = new Client(protocol);
   }
 
   @Override
-  public synchronized void createRole(String requestorUserName, String roleName)
+  public void createRole(String requestorUserName, String roleName)
     throws SentryUserException {
     TCreateSentryRoleRequest request = new TCreateSentryRoleRequest();
     request.setProtocol_version(ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT);
@@ -130,20 +111,20 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void dropRole(String requestorUserName,
+  public void dropRole(String requestorUserName,
                                     String roleName)
     throws SentryUserException {
     dropRole(requestorUserName, roleName, false);
   }
 
   @Override
-  public synchronized void dropRoleIfExists(String requestorUserName,
+  public void dropRoleIfExists(String requestorUserName,
                                             String roleName)
     throws SentryUserException {
     dropRole(requestorUserName, roleName, true);
   }
 
-  private synchronized void dropRole(String requestorUserName,
+  private void dropRole(String requestorUserName,
                                      String roleName, boolean ifExists)
     throws SentryUserException {
     TDropSentryRoleRequest request = new TDropSentryRoleRequest();
@@ -171,7 +152,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    * @throws SentryUserException
    */
   @Override
-  public synchronized Set<TSentryRole> listRolesByGroupName(
+  public Set<TSentryRole> listRolesByGroupName(
     String requestorUserName,
     String groupName)
     throws SentryUserException {
@@ -219,7 +200,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<TSentryPrivilege> listAllPrivilegesByRoleName(String requestorUserName,
+  public Set<TSentryPrivilege> listAllPrivilegesByRoleName(String requestorUserName,
                                                                         String roleName)
     throws SentryUserException {
     return listPrivilegesByRoleName(requestorUserName, roleName, null);
@@ -235,7 +216,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    * @throws SentryUserException
    */
   @Override
-  public synchronized Set<TSentryPrivilege> listPrivilegesByRoleName(String requestorUserName,
+  public Set<TSentryPrivilege> listPrivilegesByRoleName(String requestorUserName,
                                                                      String roleName, List<? extends Authorizable> authorizable)
     throws SentryUserException {
     TListSentryPrivilegesRequest request = new TListSentryPrivilegesRequest();
@@ -257,13 +238,13 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<TSentryRole> listRoles(String requestorUserName)
+  public Set<TSentryRole> listRoles(String requestorUserName)
     throws SentryUserException {
     return listRolesByGroupName(requestorUserName, null);
   }
 
   @Override
-  public synchronized Set<TSentryRole> listUserRoles(String requestorUserName)
+  public Set<TSentryRole> listUserRoles(String requestorUserName)
     throws SentryUserException {
     Set<TSentryRole> tSentryRoles = Sets.newHashSet();
     tSentryRoles.addAll(listRolesByGroupName(requestorUserName, AccessConstants.ALL));
@@ -272,7 +253,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantURIPrivilege(String requestorUserName,
+  public TSentryPrivilege grantURIPrivilege(String requestorUserName,
                                                          String roleName, String server, String uri)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName,
@@ -280,7 +261,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantURIPrivilege(String requestorUserName,
+  public TSentryPrivilege grantURIPrivilege(String requestorUserName,
                                                          String roleName, String server, String uri, Boolean grantOption)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName,
@@ -288,7 +269,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void grantServerPrivilege(String requestorUserName,
+  public void grantServerPrivilege(String requestorUserName,
                                                 String roleName, String server, String action)
     throws SentryUserException {
 
@@ -307,14 +288,14 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    * Should use grantServerPrivilege(String requestorUserName,
    *  String roleName, String server, String action, Boolean grantOption)
    */
-  public synchronized TSentryPrivilege grantServerPrivilege(String requestorUserName,
+  public TSentryPrivilege grantServerPrivilege(String requestorUserName,
                                                             String roleName, String server, Boolean grantOption) throws SentryUserException {
     return grantServerPrivilege(requestorUserName, roleName, server,
       AccessConstants.ALL, grantOption);
   }
 
   @Override
-  public synchronized TSentryPrivilege grantServerPrivilege(String requestorUserName,
+  public TSentryPrivilege grantServerPrivilege(String requestorUserName,
                                                             String roleName, String server, String action, Boolean grantOption)
     throws SentryUserException {
 
@@ -329,7 +310,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantDatabasePrivilege(String requestorUserName,
+  public TSentryPrivilege grantDatabasePrivilege(String requestorUserName,
                                                               String roleName, String server, String db, String action)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName,
@@ -337,7 +318,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantDatabasePrivilege(String requestorUserName,
+  public TSentryPrivilege grantDatabasePrivilege(String requestorUserName,
                                                               String roleName, String server, String db, String action, Boolean grantOption)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName,
@@ -345,7 +326,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantTablePrivilege(String requestorUserName,
+  public TSentryPrivilege grantTablePrivilege(String requestorUserName,
                                                            String roleName, String server, String db, String table, String action)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName, PrivilegeScope.TABLE, server,
@@ -354,7 +335,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantTablePrivilege(String requestorUserName,
+  public TSentryPrivilege grantTablePrivilege(String requestorUserName,
                                                            String roleName, String server, String db, String table, String action, Boolean grantOption)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName, PrivilegeScope.TABLE, server,
@@ -362,7 +343,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantColumnPrivilege(String requestorUserName,
+  public TSentryPrivilege grantColumnPrivilege(String requestorUserName,
                                                             String roleName, String server, String db, String table, String columnName, String action)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName, PrivilegeScope.COLUMN, server,
@@ -371,7 +352,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized TSentryPrivilege grantColumnPrivilege(String requestorUserName,
+  public TSentryPrivilege grantColumnPrivilege(String requestorUserName,
                                                             String roleName, String server, String db, String table, String columnName, String action, Boolean grantOption)
     throws SentryUserException {
     return grantPrivilege(requestorUserName, roleName, PrivilegeScope.COLUMN, server,
@@ -379,7 +360,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<TSentryPrivilege> grantColumnsPrivileges(String requestorUserName,
+  public Set<TSentryPrivilege> grantColumnsPrivileges(String requestorUserName,
                                                                    String roleName, String server, String db, String table, List<String> columnNames, String action)
     throws SentryUserException {
     return grantPrivileges(requestorUserName, roleName, PrivilegeScope.COLUMN, server,
@@ -388,7 +369,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<TSentryPrivilege> grantColumnsPrivileges(String requestorUserName,
+  public Set<TSentryPrivilege> grantColumnsPrivileges(String requestorUserName,
                                                                    String roleName, String server, String db, String table, List<String> columnNames, String action, Boolean grantOption)
     throws SentryUserException {
     return grantPrivileges(requestorUserName, roleName, PrivilegeScope.COLUMN,
@@ -397,14 +378,14 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<TSentryPrivilege> grantPrivileges(
+  public Set<TSentryPrivilege> grantPrivileges(
     String requestorUserName, String roleName,
     Set<TSentryPrivilege> privileges) throws SentryUserException {
     return grantPrivilegesCore(requestorUserName, roleName, privileges);
   }
 
   @Override
-  public synchronized TSentryPrivilege grantPrivilege(String requestorUserName, String roleName,
+  public TSentryPrivilege grantPrivilege(String requestorUserName, String roleName,
                                                       TSentryPrivilege privilege) throws SentryUserException {
     return grantPrivilegeCore(requestorUserName, roleName, privilege);
   }
@@ -500,12 +481,12 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokePrivileges(String requestorUserName, String roleName, Set<TSentryPrivilege> privileges) throws SentryUserException {
+  public void revokePrivileges(String requestorUserName, String roleName, Set<TSentryPrivilege> privileges) throws SentryUserException {
     this.revokePrivilegesCore(requestorUserName, roleName, privileges);
   }
 
   @Override
-  public synchronized void revokePrivilege(String requestorUserName, String roleName, TSentryPrivilege privilege) throws SentryUserException {
+  public void revokePrivilege(String requestorUserName, String roleName, TSentryPrivilege privilege) throws SentryUserException {
     this.revokePrivilegeCore(requestorUserName, roleName, privilege);
 
   }
@@ -530,7 +511,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeURIPrivilege(String requestorUserName,
+  public void revokeURIPrivilege(String requestorUserName,
                                               String roleName, String server, String uri)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -538,7 +519,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeURIPrivilege(String requestorUserName,
+  public void revokeURIPrivilege(String requestorUserName,
                                               String roleName, String server, String uri, Boolean grantOption)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -546,7 +527,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeServerPrivilege(String requestorUserName,
+  public void revokeServerPrivilege(String requestorUserName,
                                                  String roleName, String server, String action)
     throws SentryUserException {
 
@@ -560,7 +541,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
       PrivilegeScope.SERVER, server, null, null, null, null, action);
   }
 
-  public synchronized void revokeServerPrivilege(String requestorUserName,
+  public void revokeServerPrivilege(String requestorUserName,
                                                  String roleName, String server, String action, Boolean grantOption)
     throws SentryUserException {
 
@@ -580,7 +561,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    *  String roleName, String server, String action, Boolean grantOption)
    */
   @Override
-  public synchronized void revokeServerPrivilege(String requestorUserName,
+  public void revokeServerPrivilege(String requestorUserName,
                                                  String roleName, String server, boolean grantOption)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -588,7 +569,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeDatabasePrivilege(String requestorUserName,
+  public void revokeDatabasePrivilege(String requestorUserName,
                                                    String roleName, String server, String db, String action)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -596,7 +577,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeDatabasePrivilege(String requestorUserName,
+  public void revokeDatabasePrivilege(String requestorUserName,
                                                    String roleName, String server, String db, String action, Boolean grantOption)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -604,7 +585,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeTablePrivilege(String requestorUserName,
+  public void revokeTablePrivilege(String requestorUserName,
                                                 String roleName, String server, String db, String table, String action)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -613,7 +594,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeTablePrivilege(String requestorUserName,
+  public void revokeTablePrivilege(String requestorUserName,
                                                 String roleName, String server, String db, String table, String action, Boolean grantOption)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -622,7 +603,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeColumnPrivilege(String requestorUserName, String roleName,
+  public void revokeColumnPrivilege(String requestorUserName, String roleName,
                                                  String server, String db, String table, String columnName, String action)
     throws SentryUserException {
     ImmutableList.Builder<String> listBuilder = ImmutableList.builder();
@@ -633,7 +614,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeColumnPrivilege(String requestorUserName, String roleName,
+  public void revokeColumnPrivilege(String requestorUserName, String roleName,
                                                  String server, String db, String table, String columnName, String action, Boolean grantOption)
     throws SentryUserException {
     ImmutableList.Builder<String> listBuilder = ImmutableList.builder();
@@ -644,7 +625,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeColumnsPrivilege(String requestorUserName, String roleName,
+  public void revokeColumnsPrivilege(String requestorUserName, String roleName,
                                                   String server, String db, String table, List<String> columns, String action)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -653,7 +634,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeColumnsPrivilege(String requestorUserName, String roleName,
+  public void revokeColumnsPrivilege(String requestorUserName, String roleName,
                                                   String server, String db, String table, List<String> columns, String action, Boolean grantOption)
     throws SentryUserException {
     revokePrivilege(requestorUserName, roleName,
@@ -741,7 +722,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Set<String> listPrivilegesForProvider
+  public Set<String> listPrivilegesForProvider
     (Set<String> groups, Set<String> users,
      ActiveRoleSet roleSet, Authorizable... authorizable) throws SentryUserException {
     TSentryActiveRoleSet thriftRoleSet = new TSentryActiveRoleSet(roleSet.isAll(), roleSet.getRoles());
@@ -766,21 +747,21 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void grantRoleToGroup(String requestorUserName,
+  public void grantRoleToGroup(String requestorUserName,
                                             String groupName, String roleName)
     throws SentryUserException {
     grantRoleToGroups(requestorUserName, roleName, Sets.newHashSet(groupName));
   }
 
   @Override
-  public synchronized void revokeRoleFromGroup(String requestorUserName,
+  public void revokeRoleFromGroup(String requestorUserName,
                                                String groupName, String roleName)
     throws SentryUserException {
     revokeRoleFromGroups(requestorUserName, roleName, Sets.newHashSet(groupName));
   }
 
   @Override
-  public synchronized void grantRoleToGroups(String requestorUserName,
+  public void grantRoleToGroups(String requestorUserName,
                                              String roleName, Set<String> groups)
     throws SentryUserException {
     TAlterSentryRoleAddGroupsRequest request = new TAlterSentryRoleAddGroupsRequest(
@@ -795,7 +776,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeRoleFromGroups(String requestorUserName,
+  public void revokeRoleFromGroups(String requestorUserName,
                                                 String roleName, Set<String> groups)
     throws SentryUserException {
     TAlterSentryRoleDeleteGroupsRequest request = new TAlterSentryRoleDeleteGroupsRequest(
@@ -810,19 +791,19 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void grantRoleToUser(String requestorUserName, String userName,
+  public void grantRoleToUser(String requestorUserName, String userName,
                                            String roleName) throws SentryUserException {
     grantRoleToUsers(requestorUserName, roleName, Sets.newHashSet(userName));
   }
 
   @Override
-  public synchronized void revokeRoleFromUser(String requestorUserName, String userName,
+  public void revokeRoleFromUser(String requestorUserName, String userName,
                                               String roleName) throws SentryUserException {
     revokeRoleFromUsers(requestorUserName, roleName, Sets.newHashSet(userName));
   }
 
   @Override
-  public synchronized void grantRoleToUsers(String requestorUserName, String roleName,
+  public void grantRoleToUsers(String requestorUserName, String roleName,
                                             Set<String> users) throws SentryUserException {
     TAlterSentryRoleAddUsersRequest request = new TAlterSentryRoleAddUsersRequest(
       ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName, roleName, users);
@@ -835,7 +816,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void revokeRoleFromUsers(String requestorUserName, String roleName,
+  public void revokeRoleFromUsers(String requestorUserName, String roleName,
                                                Set<String> users) throws SentryUserException {
     TAlterSentryRoleDeleteUsersRequest request = new TAlterSentryRoleDeleteUsersRequest(
       ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName, roleName, users);
@@ -858,7 +839,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void dropPrivileges(String requestorUserName,
+  public void dropPrivileges(String requestorUserName,
                                           List<? extends Authorizable> authorizableObjects)
     throws SentryUserException {
     TSentryAuthorizable tSentryAuthorizable = setupSentryAuthorizable(authorizableObjects);
@@ -875,7 +856,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void renamePrivileges(String requestorUserName,
+  public void renamePrivileges(String requestorUserName,
                                             List<? extends Authorizable> oldAuthorizables,
                                             List<? extends Authorizable> newAuthorizables) throws SentryUserException {
     TSentryAuthorizable tOldSentryAuthorizable = setupSentryAuthorizable(oldAuthorizables);
@@ -894,7 +875,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized Map<TSentryAuthorizable, TSentryPrivilegeMap> listPrivilegsbyAuthorizable
+  public Map<TSentryAuthorizable, TSentryPrivilegeMap> listPrivilegsbyAuthorizable
     (
       String requestorUserName,
       Set<List<? extends Authorizable>> authorizables, Set<String> groups,
@@ -937,7 +918,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    */
 
   @Override
-  public synchronized String getConfigValue(String propertyName, String defaultValue)
+  public String getConfigValue(String propertyName, String defaultValue)
     throws SentryUserException {
     TSentryConfigValueRequest request = new TSentryConfigValueRequest(
       ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT, propertyName);
@@ -977,7 +958,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
    * @param requestorUserName     The name of the request user
    */
   @Override
-  public synchronized void importPolicy
+  public void importPolicy
   (Map<String, Map<String, Set<String>>> policyFileMappingData,
    String requestorUserName, boolean isOverwriteRole)
     throws SentryUserException {
@@ -1022,7 +1003,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
 
   // export the sentry mapping data with map structure
   @Override
-  public synchronized Map<String, Map<String, Set<String>>> exportPolicy(String
+  public Map<String, Map<String, Set<String>>> exportPolicy(String
                                                                            requestorUserName,
                                                                          String objectPath) throws SentryUserException {
     TSentryExportMappingDataRequest request = new TSentryExportMappingDataRequest(
@@ -1065,12 +1046,23 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
   }
 
   @Override
-  public synchronized void close() {
-    transportFactory.close();
+  public void close() {
+    done();
+  }
+
+  @Override
+  public void done() {
+    if (transport != null) {
+      transportPool.returnTransport(transport);
+      transport = null;
+    }
   }
 
   @Override
-  public void disconnect() {
-    transportFactory.releaseTransport();
+  public void invalidate() {
+    if (transport != null) {
+      transportPool.invalidateTransport(transport);
+      transport = null;
+    }
   }
 }
\ No newline at end of file


Mime
View raw message