sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gcha...@apache.org
Subject sentry git commit: SENTRY-1229: Add caching to SentryGenericProviderBackend (Ashish K Singh, Reviewed By: Gregory Chanan)
Date Mon, 16 May 2016 02:16:27 GMT
Repository: sentry
Updated Branches:
  refs/heads/master 1cbf44ade -> eceaaf8e0


SENTRY-1229: Add caching to SentryGenericProviderBackend (Ashish K Singh, Reviewed By: Gregory
Chanan)


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

Branch: refs/heads/master
Commit: eceaaf8e0f83930a28e81d9380e5f75e3ebc4019
Parents: 1cbf44a
Author: Gregory Chanan <gchanan@cloudera.com>
Authored: Sun May 15 19:14:57 2016 -0700
Committer: Gregory Chanan <gchanan@cloudera.com>
Committed: Sun May 15 19:14:57 2016 -0700

----------------------------------------------------------------------
 .../sentry/provider/common/CacheProvider.java   |  68 ++++++++
 .../sentry/provider/common/ProviderBackend.java |   2 +-
 .../sentry/provider/common/TableCache.java      |  25 +++
 .../generic/SentryGenericProviderBackend.java   | 118 ++++++++-----
 .../provider/db/generic/UpdatableCache.java     | 171 +++++++++++++++++++
 .../thrift/SentryGenericServiceClient.java      |   3 +-
 .../SentryGenericServiceClientDefaultImpl.java  |   1 -
 .../sentry/service/thrift/ServiceConstants.java |   9 +
 .../file/SimpleFileProviderBackend.java         | 105 +++---------
 .../e2e/kafka/AbstractKafkaSentryTestBase.java  |  13 +-
 .../sentry/tests/e2e/kafka/TestAuthorize.java   |   1 +
 11 files changed, 393 insertions(+), 123 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java
new file mode 100644
index 0000000..d50a0bc
--- /dev/null
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java
@@ -0,0 +1,68 @@
+/**
+ * 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.common;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.sentry.core.common.ActiveRoleSet;
+import org.apache.sentry.core.common.Authorizable;
+
+import java.util.Map;
+import java.util.Set;
+
+public class CacheProvider {
+  private TableCache cache;
+  private volatile boolean initialized = false;
+
+  public void initialize(TableCache cache) {
+    if (initialized) {
+      throw new IllegalStateException("CacheProvider has already been initialized, cannot
be initialized twice.");
+    }
+    this.cache = cache;
+    this.initialized = true;
+  }
+
+  public ImmutableSet<String> getPrivileges(Set<String> groups, ActiveRoleSet
roleSet,
+                                            Authorizable... authorizableHierarchy) {
+    if (!initialized) {
+      throw new IllegalStateException("CacheProvider has not been properly initialized");
+    }
+    ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder();
+    for (String groupName : groups) {
+      for (Map.Entry<String, Set<String>> row : cache.getCache().row(groupName).entrySet())
{
+        if (roleSet.containsRole(row.getKey())) {
+          // TODO: SENTRY-1245: Filter by Authorizables, if provided
+          resultBuilder.addAll(row.getValue());
+        }
+      }
+    }
+    return resultBuilder.build();
+  }
+
+  public ImmutableSet<String> getRoles(Set<String> groups, ActiveRoleSet roleSet)
{
+    if (!initialized) {
+      throw new IllegalStateException("CacheProvider has not been properly initialized");
+    }
+    ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder();
+    if (groups != null) {
+      for (String groupName : groups) {
+        for (Map.Entry<String, Set<String>> row : cache.getCache().row(groupName)
+            .entrySet()) {
+          if (roleSet.containsRole(row.getKey())) {
+            resultBuilder.add(row.getKey());
+          }
+        }
+      }
+    }
+    return resultBuilder.build();
+  }
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
index ffd3af4..c78718c 100644
--- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
@@ -40,7 +40,7 @@ public interface ProviderBackend {
    * policy engine knows the validators. Ideally we could change but since
    * both the policy engine and backend are exposed via configuration properties
    * that would be backwards incompatible.
-   * @param validators
+   * @param context
    */
   void initialize(ProviderBackendContext context);
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java
new file mode 100644
index 0000000..3285c1b
--- /dev/null
+++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java
@@ -0,0 +1,25 @@
+/**
+ * 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.provider.common;
+
+import com.google.common.collect.Table;
+
+import java.util.Set;
+
+public interface TableCache {
+  /**
+   * Returns backing cache. Caller must not modify the returned cache.
+   * @return backing cache.
+   */
+  Table<String, String, Set<String>> getCache();
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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 222b6fd..6de0a54 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
@@ -17,6 +17,8 @@
  */
 package org.apache.sentry.provider.db.generic;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.Set;
 
@@ -26,11 +28,14 @@ import org.apache.sentry.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
 import org.apache.sentry.core.common.SentryConfigurationException;
+import org.apache.sentry.provider.common.CacheProvider;
 import org.apache.sentry.provider.common.ProviderBackend;
 import org.apache.sentry.provider.common.ProviderBackendContext;
 import org.apache.sentry.provider.db.generic.service.thrift.SentryGenericServiceClient;
 import org.apache.sentry.provider.db.generic.service.thrift.SentryGenericServiceClientFactory;
 import org.apache.sentry.provider.db.generic.service.thrift.TSentryRole;
+import org.apache.sentry.provider.db.generic.tools.command.TSentryPrivilegeConvertor;
+import org.apache.sentry.service.thrift.ServiceConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,18 +45,22 @@ import com.google.common.collect.Sets;
 /**
  * This class used when any component such as Hive, Solr or Sqoop want to integration with
the Sentry service
  */
-public class SentryGenericProviderBackend implements ProviderBackend {
+public class SentryGenericProviderBackend extends CacheProvider implements ProviderBackend
{
   private static final Logger LOGGER = LoggerFactory.getLogger(SentryGenericProviderBackend.class);
   private final Configuration conf;
   private volatile boolean initialized = false;
   private String componentType;
   private String serviceName;
+  private boolean enableCaching;
+  private String privilegeConverter;
 
   // ProviderBackend should have the same construct to support the reflect in authBinding,
   // eg:SqoopAuthBinding
   public SentryGenericProviderBackend(Configuration conf, String resource) //NOPMD
       throws Exception {
     this.conf = conf;
+    this.enableCaching = conf.getBoolean(ServiceConstants.ClientConfig.ENABLE_CACHING, ServiceConstants.ClientConfig.ENABLE_CACHING_DEFAULT);
+    this.privilegeConverter = conf.get(ServiceConstants.ClientConfig.PRIVILEGE_CONVERTER);
   }
 
   @Override
@@ -59,6 +68,28 @@ public class SentryGenericProviderBackend implements ProviderBackend {
     if (initialized) {
       throw new IllegalStateException("SentryGenericProviderBackend has already been initialized,
cannot be initialized twice");
     }
+    if (enableCaching) {
+      if (privilegeConverter == null) {
+        throw new SentryConfigurationException(ServiceConstants.ClientConfig.PRIVILEGE_CONVERTER
+ " not configured.");
+      }
+
+      Constructor<?> privilegeConverterConstructor;
+      TSentryPrivilegeConvertor sentryPrivilegeConvertor;
+      try {
+        privilegeConverterConstructor = Class.forName(privilegeConverter).getDeclaredConstructor(String.class,
String.class);
+        privilegeConverterConstructor.setAccessible(true);
+        sentryPrivilegeConvertor = (TSentryPrivilegeConvertor) privilegeConverterConstructor.newInstance(getComponentType(),
getServiceName());
+      } catch (NoSuchMethodException | ClassNotFoundException | InstantiationException |
InvocationTargetException | IllegalAccessException e) {
+        throw new RuntimeException("Failed to create privilege converter of type " + privilegeConverter,
e);
+      }
+      UpdatableCache cache = new UpdatableCache(conf, getComponentType(), getServiceName(),
sentryPrivilegeConvertor);
+      try {
+        cache.startUpdateThread(true);
+      } catch (Exception e) {
+        throw new RuntimeException("Failed to get privileges from Sentry to build cache.",
e);
+      }
+      super.initialize(cache);
+    }
     this.initialized = true;
   }
 
@@ -76,20 +107,24 @@ public class SentryGenericProviderBackend implements ProviderBackend
{
     if (!initialized) {
       throw new IllegalStateException("SentryGenericProviderBackend has not been properly
initialized");
     }
-    SentryGenericServiceClient client = null;
-    try {
-      client = getClient();
-      return ImmutableSet.copyOf(client.listPrivilegesForProvider(componentType, serviceName,
-          roleSet, groups, Arrays.asList(authorizableHierarchy)));
-    } catch (SentryUserException e) {
-      String msg = "Unable to obtain privileges from server: " + e.getMessage();
-      LOGGER.error(msg, e);
-    } catch (Exception e) {
-      String msg = "Unable to obtain client:" + e.getMessage();
-      LOGGER.error(msg, e);
-    } finally {
-      if (client != null) {
-        client.close();
+    if (enableCaching) {
+      return super.getPrivileges(groups, roleSet, authorizableHierarchy);
+    } else {
+      SentryGenericServiceClient client = null;
+      try {
+        client = getClient();
+        return ImmutableSet.copyOf(client.listPrivilegesForProvider(componentType, serviceName,
+            roleSet, groups, Arrays.asList(authorizableHierarchy)));
+      } catch (SentryUserException e) {
+        String msg = "Unable to obtain privileges from server: " + e.getMessage();
+        LOGGER.error(msg, e);
+      } catch (Exception e) {
+        String msg = "Unable to obtain client:" + e.getMessage();
+        LOGGER.error(msg, e);
+      } finally {
+        if (client != null) {
+          client.close();
+        }
       }
     }
     return ImmutableSet.of();
@@ -100,32 +135,36 @@ public class SentryGenericProviderBackend implements ProviderBackend
{
     if (!initialized) {
       throw new IllegalStateException("SentryGenericProviderBackend has not been properly
initialized");
     }
-    SentryGenericServiceClient client = null;
-    try {
-      Set<TSentryRole> tRoles = Sets.newHashSet();
-      client = getClient();
-      //get the roles according to group
-      String requestor = UserGroupInformation.getCurrentUser().getShortUserName();
-      for (String group : groups) {
-        tRoles.addAll(client.listRolesByGroupName(requestor, group, getComponentType()));
-      }
-      Set<String> roles = Sets.newHashSet();
-      for (TSentryRole tRole : tRoles) {
-        roles.add(tRole.getRoleName());
-      }
-      return ImmutableSet.copyOf(roleSet.isAll() ? roles : Sets.intersection(roles, roleSet.getRoles()));
-    } catch (SentryUserException e) {
-      String msg = "Unable to obtain roles from server: " + e.getMessage();
-      LOGGER.error(msg, e);
-    } catch (Exception e) {
-      String msg = "Unable to obtain client:" + e.getMessage();
-      LOGGER.error(msg, e);
-    } finally {
-      if (client != null) {
-        client.close();
+    if (enableCaching) {
+      return super.getRoles(groups, roleSet);
+    } else {
+      SentryGenericServiceClient client = null;
+      try {
+        Set<TSentryRole> tRoles = Sets.newHashSet();
+        client = getClient();
+        //get the roles according to group
+        String requestor = UserGroupInformation.getCurrentUser().getShortUserName();
+        for (String group : groups) {
+          tRoles.addAll(client.listRolesByGroupName(requestor, group, getComponentType()));
+        }
+        Set<String> roles = Sets.newHashSet();
+        for (TSentryRole tRole : tRoles) {
+          roles.add(tRole.getRoleName());
+        }
+        return ImmutableSet.copyOf(roleSet.isAll() ? roles : Sets.intersection(roles, roleSet.getRoles()));
+      } catch (SentryUserException e) {
+        String msg = "Unable to obtain roles from server: " + e.getMessage();
+        LOGGER.error(msg, e);
+      } catch (Exception e) {
+        String msg = "Unable to obtain client:" + e.getMessage();
+        LOGGER.error(msg, e);
+      } finally {
+        if (client != null) {
+          client.close();
+        }
       }
+      return ImmutableSet.of();
     }
-    return ImmutableSet.of();
   }
 
   /**
@@ -165,5 +204,4 @@ public class SentryGenericProviderBackend implements ProviderBackend {
   public void setServiceName(String serviceName) {
     this.serviceName = serviceName;
   }
-
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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
new file mode 100644
index 0000000..ccb349b
--- /dev/null
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
@@ -0,0 +1,171 @@
+/**
+ * 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.generic;
+
+import com.google.common.collect.Table;
+import com.google.common.collect.HashBasedTable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.sentry.provider.common.TableCache;
+import org.apache.sentry.provider.db.generic.service.thrift.*;
+import org.apache.sentry.provider.db.generic.tools.command.TSentryPrivilegeConvertor;
+import org.apache.sentry.service.thrift.ServiceConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.concurrent.TimeUnit;
+
+class UpdatableCache implements TableCache {
+  private static final Logger LOGGER = LoggerFactory.getLogger(UpdatableCache.class);
+
+  private final String componentType;
+  private final String serviceName;
+  private final long cacheTtlNs;
+  private final int allowedUpdateFailuresCount;
+  private final Configuration conf;
+  private final TSentryPrivilegeConvertor tSentryPrivilegeConvertor;
+
+  private volatile long lastRefreshedNs = 0;
+  private int consecutiveUpdateFailuresCount = 0;
+  /**
+   * Sparse table where group is the row key and role is the cell.
+   * The value is the set of privileges located in the cell. For example,
+   * the following table would be generated for a policy where Group 1
+   * has Role 1 and Role 2 while Group 2 has only Role 2.
+   * <table border="1">
+   *  <tbody>
+   *    <tr>
+   *      <td><!-- empty --></td>
+   *      <td>Role 1</td>
+   *      <td>Role 2</td>
+   *    </tr>
+   *    <tr>
+   *      <td>Group 1</td>
+   *      <td>Priv 1</td>
+   *      <td>Priv 2, Priv 3</td>
+   *    </tr>
+   *    <tr>
+   *      <td>Group 2</td>
+   *      <td><!-- empty --></td>
+   *      <td>Priv 2, Priv 3</td>
+   *    </tr>
+   *  </tbody>
+   * </table>
+   */
+  private volatile Table<String, String, Set<String>> table;
+
+  UpdatableCache(Configuration conf, String componentType, String serviceName, TSentryPrivilegeConvertor
tSentryPrivilegeConvertor) {
+    this.conf = conf;
+    this.componentType = componentType;
+    this.serviceName = serviceName;
+    this.tSentryPrivilegeConvertor = tSentryPrivilegeConvertor;
+
+    // check caching configuration
+    this.cacheTtlNs = TimeUnit.MILLISECONDS.toNanos(conf.getLong(ServiceConstants.ClientConfig.CACHE_TTL_MS,
ServiceConstants.ClientConfig.CACHING_TTL_MS_DEFAULT));
+    this.allowedUpdateFailuresCount = conf.getInt(ServiceConstants.ClientConfig.CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE,
ServiceConstants.ClientConfig.CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE_DEFAULT);
+  }
+
+  @Override
+  public Table<String, String, Set<String>> getCache() {
+    return table;
+  }
+
+  /**
+   * Build cache replica with latest values
+   *
+   * @return cache replica with latest values
+   */
+  private Table<String, String, Set<String>> loadFromRemote() throws Exception
{
+    Table<String, String, Set<String>> tempCache = HashBasedTable.create();
+    String requestor;
+    requestor = UserGroupInformation.getLoginUser().getShortUserName();
+
+    final SentryGenericServiceClient client = getClient();
+    final 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);
+      for (String group : tSentryRole.getGroups()) {
+        Set<String> currentPrivileges = tempCache.get(group, roleName);
+        if (currentPrivileges == null) {
+          currentPrivileges = new HashSet<>();
+          tempCache.put(group, roleName, currentPrivileges);
+        }
+        for (TSentryPrivilege tSentryPrivilege : tSentryPrivileges) {
+          currentPrivileges.add(tSentryPrivilegeConvertor.toString(tSentryPrivilege));
+        }
+      }
+    }
+    return tempCache;
+  }
+
+  /**
+   *  The Sentry-296(generate client for connection pooling) has already finished development
and reviewed by now. When it
+   *  was committed to master, the getClient method was needed to refactor using the connection
pool
+   *
+   *  TODO: Avoid creating new client each time.
+   */
+  private SentryGenericServiceClient getClient() throws Exception {
+    return SentryGenericServiceClientFactory.create(conf);
+  }
+
+  void startUpdateThread(boolean blockUntilFirstReload) throws Exception {
+    if (blockUntilFirstReload) {
+      reloadData();
+    }
+
+    Timer timer = new Timer();
+    final long refreshIntervalMs = TimeUnit.NANOSECONDS.toMillis(cacheTtlNs);
+    timer.scheduleAtFixedRate(
+        new TimerTask() {
+          public void run() {
+            if (shouldRefresh()) {
+              try {
+                LOGGER.debug("Loading all data.");
+                reloadData();
+              } catch (Exception e) {
+                LOGGER.warn("Exception while updating data from DB", e);
+                revokeAllPrivilegesIfRequired();
+              }
+            }
+          }
+        },
+        blockUntilFirstReload ? refreshIntervalMs : 0,
+        refreshIntervalMs);
+  }
+
+  private void revokeAllPrivilegesIfRequired() {
+    if (++consecutiveUpdateFailuresCount > allowedUpdateFailuresCount) {
+      // 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();
+      LOGGER.error("Failed to update roles and privileges cache for " + consecutiveUpdateFailuresCount
+ " times." +
+          " Revoking all privileges from cache, which will cause all authorization requests
to fail.");
+    }
+  }
+
+  private void reloadData() throws Exception {
+    this.table = loadFromRemote();
+    lastRefreshedNs = System.nanoTime();
+  }
+
+  private boolean shouldRefresh() {
+    final long currentTimeNs = System.nanoTime();
+    return lastRefreshedNs + cacheTtlNs < currentTimeNs;
+  }
+}

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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 76ff15b..bf87d8b 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
@@ -100,7 +100,6 @@ public interface SentryGenericServiceClient {
   /**
    * drop privilege
    * @param requestorUserName: user on whose behalf the request is issued
-   * @param roleName: Name of the role
    * @param component: The request is issued to which component
    * @param privilege
    * @throws SentryUserException
@@ -142,7 +141,7 @@ public interface SentryGenericServiceClient {
       throws SentryUserException;
 
   /**
-   * Gets sentry privileges for a given roleName and Authorizable Hirerchys using the Sentry
service
+   * Gets sentry privileges for a given roleName and Authorizable Hierarchy using the Sentry
service
    * @param requestorUserName: user on whose behalf the request is issued
    * @param roleName:
    * @param component: The request is issued to which component

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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 74b6963..744dcd7 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
@@ -350,7 +350,6 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi
   /**
    * drop privilege
    * @param requestorUserName: user on whose behalf the request is issued
-   * @param roleName: Name of the role
    * @param component: The request is issued to which component
    * @param privilege
    * @throws SentryUserException

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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 00e3fbd..d7ccc45 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
@@ -226,6 +226,15 @@ public class ServiceConstants {
     public static final int RETRY_COUNT_DEFAULT = 3;
     public static final String RETRY_INTERVAL_SEC_CONF = "sentry.provider.backend.db.retry.interval.seconds";
     public static final int RETRY_INTERVAL_SEC_DEFAULT = 30;
+
+    // provider backend cache settings
+    public static final String ENABLE_CACHING = "sentry.provider.backend.generic.cache.enabled";
+    public static final boolean ENABLE_CACHING_DEFAULT = false;
+    public static final String CACHE_TTL_MS = "sentry.provider.backend.generic.cache.ttl.ms";
+    public static final long CACHING_TTL_MS_DEFAULT = 30000;
+    public static final String CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE = "sentry.provider.backend.generic.cache.update.failures.count";
+    public static final int CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE_DEFAULT = 3;
+    public static final String PRIVILEGE_CONVERTER = "sentry.provider.backend.generic.privilege.converter";
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
index 079d52a..2a64621 100644
--- a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
+++ b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
@@ -37,8 +37,10 @@ import org.apache.sentry.policy.common.PrivilegeUtils;
 import org.apache.sentry.core.common.validator.PrivilegeValidator;
 import org.apache.sentry.core.common.validator.PrivilegeValidatorContext;
 import org.apache.sentry.core.common.utils.PolicyFileConstants;
+import org.apache.sentry.provider.common.CacheProvider;
 import org.apache.sentry.provider.common.ProviderBackend;
 import org.apache.sentry.provider.common.ProviderBackendContext;
+import org.apache.sentry.provider.common.TableCache;
 import org.apache.shiro.config.Ini;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -57,7 +59,7 @@ import com.google.common.collect.Sets;
 import com.google.common.collect.Table;
 import com.google.common.collect.Table.Cell;
 
-public class SimpleFileProviderBackend implements ProviderBackend {
+public class SimpleFileProviderBackend extends CacheProvider implements ProviderBackend {
 
   private static final Logger LOGGER = LoggerFactory
       .getLogger(SimpleFileProviderBackend.class);
@@ -67,33 +69,7 @@ public class SimpleFileProviderBackend implements ProviderBackend {
   private final Configuration conf;
   private final List<String> configErrors;
   private final List<String> configWarnings;
-
-  /**
-   * Sparse table where group is the row key and role is the cell.
-   * The value is the set of privileges located in the cell. For example,
-   * the following table would be generated for a policy where Group 1
-   * has Role 1 and Role 2 while Group 2 has only Role 2.
-   * <table border="1">
-   *  <tbody>
-   *    <tr>
-   *      <td><!-- empty --></td>
-   *      <td>Role 1</td>
-   *      <td>Role 2</td>
-   *    </tr>
-   *    <tr>
-   *      <td>Group 1</td>
-   *      <td>Priv 1</td>
-   *      <td>Priv 2, Priv 3</td>
-   *    </tr>
-   *    <tr>
-   *      <td>Group 2</td>
-   *      <td><!-- empty --></td>
-   *      <td>Priv 2, Priv 3</td>
-   *    </tr>
-   *  </tbody>
-   * </table>
-   */
-  private final Table<String, String, Set<String>> groupRolePrivilegeTable;
+  private TableCache cache;
   /**
    * Each group, role, and privilege in groupRolePrivilegeTable is
    * interned using a weak interner so that we only store each string
@@ -112,7 +88,6 @@ public class SimpleFileProviderBackend implements ProviderBackend {
   public SimpleFileProviderBackend(Configuration conf, Path resourcePath) throws IOException
{
     this.resourcePath = resourcePath;
     this.fileSystem = resourcePath.getFileSystem(conf);
-    this.groupRolePrivilegeTable = HashBasedTable.create();
     this.conf = conf;
     this.configErrors = Lists.newArrayList();
     this.configWarnings = Lists.newArrayList();
@@ -132,28 +107,15 @@ public class SimpleFileProviderBackend implements ProviderBackend {
     }
     this.validators = context.getValidators();
     this.allowPerDatabaseSection = context.isAllowPerDatabase();
-    parse();
-    this.initialized = true;
-  }
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public ImmutableSet<String> getPrivileges(Set<String> groups, ActiveRoleSet
roleSet, Authorizable... authorizableHierarchy) {
-    if (!initialized) {
-      throw new IllegalStateException("Backend has not been properly initialized");
-    }
-    ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder();
-    for (String groupName : groups) {
-      for (Map.Entry<String, Set<String>> row : groupRolePrivilegeTable.row(groupName)
-          .entrySet()) {
-        if (roleSet.containsRole(row.getKey())) {
-          resultBuilder.addAll(row.getValue());
-        }
+    final Table<String, String, Set<String>> table = parse();
+    this.cache = new TableCache() {
+      @Override
+      public Table<String, String, Set<String>> getCache() {
+        return table;
       }
-    }
-    return resultBuilder.build();
+    };
+    super.initialize(cache);
+    this.initialized = true;
   }
 
   @Override
@@ -163,28 +125,6 @@ public class SimpleFileProviderBackend implements ProviderBackend {
     return getPrivileges(groups, roleSet, authorizableHierarchy);
   }
 
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public ImmutableSet<String> getRoles(Set<String> groups, ActiveRoleSet roleSet)
{
-    if (!initialized) {
-      throw new IllegalStateException("Backend has not been properly initialized");
-    }
-    ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder();
-    if (groups != null) {
-      for (String groupName : groups) {
-        for (Map.Entry<String, Set<String>> row : groupRolePrivilegeTable.row(groupName)
-            .entrySet()) {
-          if (roleSet.containsRole(row.getKey())) {
-            resultBuilder.add(row.getKey());
-          }
-        }
-      }
-    }
-    return resultBuilder.build();
-  }
-
   @Override
   public void close() {
     // SENTRY-847 will use HiveAuthBinding again, so groupRolePrivilegeTable shouldn't clear
itself
@@ -206,9 +146,10 @@ public class SimpleFileProviderBackend implements ProviderBackend {
     }
   }
 
-  private void parse() {
+  private Table<String, String, Set<String>> parse() {
     configErrors.clear();
     configWarnings.clear();
+    Table<String, String, Set<String>> groupRolePrivilegeTable = HashBasedTable.create();
     Table<String, String, Set<String>> groupRolePrivilegeTableTemp = HashBasedTable.create();
     Ini ini;
     LOGGER.info("Parsing " + resourcePath);
@@ -237,7 +178,7 @@ public class SimpleFileProviderBackend implements ProviderBackend {
         }
       }
       parseIni(null, ini, validators, resourcePath, groupRolePrivilegeTableTemp);
-      mergeResult(groupRolePrivilegeTableTemp);
+      mergeResult(groupRolePrivilegeTable, groupRolePrivilegeTableTemp);
       groupRolePrivilegeTableTemp.clear();
       Ini.Section filesSection = ini.getSection(PolicyFileConstants.DATABASES);
       if(filesSection == null) {
@@ -265,6 +206,8 @@ public class SimpleFileProviderBackend implements ProviderBackend {
               throw new SentryConfigurationException("Per-db policy files cannot contain
" + PolicyFileConstants.DATABASES + " section");
             }
             parseIni(database, perDbIni, validators, perDbPolicy, groupRolePrivilegeTableTemp);
+            mergeResult(groupRolePrivilegeTable, groupRolePrivilegeTableTemp);
+            groupRolePrivilegeTableTemp.clear();
           } catch (Exception e) {
             configErrors.add("Failed to read per-DB policy file " + perDbPolicy +
                " Error: " + e.getMessage());
@@ -272,12 +215,12 @@ public class SimpleFileProviderBackend implements ProviderBackend {
           }
         }
       }
-      mergeResult(groupRolePrivilegeTableTemp);
-      groupRolePrivilegeTableTemp.clear();
     } catch (Exception e) {
       configErrors.add("Error processing file " + resourcePath + ".  Message: " + e.getMessage());
       LOGGER.error("Error processing file, ignoring " + resourcePath, e);
     }
+
+    return groupRolePrivilegeTable;
   }
 
   /**
@@ -289,7 +232,8 @@ public class SimpleFileProviderBackend implements ProviderBackend {
     return uri.getAuthority() == null && uri.getScheme() == null && !path.isUriPathAbsolute();
   }
 
-  private void mergeResult(Table<String, String, Set<String>> groupRolePrivilegeTableTemp)
{
+  private void mergeResult(Table<String, String, Set<String>> groupRolePrivilegeTable,
+                           Table<String, String, Set<String>> groupRolePrivilegeTableTemp)
{
     for (Cell<String, String, Set<String>> cell : groupRolePrivilegeTableTemp.cellSet())
{
       String groupName = cell.getRowKey();
       String roleName = cell.getColumnKey();
@@ -387,7 +331,12 @@ public class SimpleFileProviderBackend implements ProviderBackend {
     }
   }
 
+  /**
+   * Returns backing table of group-role-privileges cache.
+   * Caller must not modify the returned table.
+   * @return backing table of cache.
+   */
   public Table<String, String, Set<String>> getGroupRolePrivilegeTable() {
-    return groupRolePrivilegeTable;
+    return this.cache.getCache();
   }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
index a2cfa28..c4e3863 100644
--- a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
+++ b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
@@ -61,6 +61,8 @@ public class AbstractKafkaSentryTestBase {
   protected static final String ADMIN_USER = "kafka";
   protected static final String ADMIN_GROUP = "group_kafka";
   protected static final String ADMIN_ROLE  = "role_kafka";
+  private static final long CACHE_TTL_MS = 1;
+  private static final int SAFETY_FACTOR = 2; // Sleep for specified times of expected time
for an operation to complete.
 
   protected static SentryService sentryServer;
   protected static File sentrySitePath;
@@ -205,13 +207,16 @@ public class AbstractKafkaSentryTestBase {
     /** set the Sentry client configuration for Kafka Service integration */
     conf.set(ServerConfig.SECURITY_MODE, ServerConfig.SECURITY_MODE_NONE);
     conf.set(ClientConfig.SERVER_RPC_ADDRESS, sentryServer.getAddress().getHostName());
-    conf.set(ClientConfig.SERVER_RPC_PORT, String.valueOf(sentryServer.getAddress().getPort()));
+    conf.setInt(ClientConfig.SERVER_RPC_PORT, sentryServer.getAddress().getPort());
 
     conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER.getVar(),
         LocalGroupResourceAuthorizationProvider.class.getName());
     conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER_BACKEND.getVar(),
         SentryGenericProviderBackend.class.getName());
     conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER_RESOURCE.getVar(), policyFilePath.getPath());
+    conf.setBoolean(ClientConfig.ENABLE_CACHING, true);
+    conf.setLong(ClientConfig.CACHE_TTL_MS, CACHE_TTL_MS);
+    conf.set(ClientConfig.PRIVILEGE_CONVERTER, "org.apache.sentry.provider.db.generic.tools.KafkaTSentryPrivilegeConvertor");
     return conf;
   }
 
@@ -224,4 +229,10 @@ public class AbstractKafkaSentryTestBase {
     kafkaServer.start();
     bootstrapServers = kafkaServer.getBootstrapServers();
   }
+
+  static void sleepIfCachingEnabled() throws InterruptedException {
+    if (getClientConfig().getBoolean(ClientConfig.ENABLE_CACHING, false)) {
+      Thread.sleep(CACHE_TTL_MS * SAFETY_FACTOR);
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
index 250522e..6017451 100644
--- a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
+++ b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java
@@ -173,6 +173,7 @@ public class TestAuthorize extends AbstractKafkaSentryTestBase {
         sentryClient = null;
       }
     }
+    sleepIfCachingEnabled();
   }
 
   private void testProduce(String producerUser) throws Exception {


Mime
View raw message