You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by va...@apache.org on 2017/06/14 00:57:20 UTC

[42/52] [abbrv] sentry git commit: SENTRY-1773: Delta change cleaner should leave way more then a single entry intact (Alex Kolbasov, reviewed by Vamsee Yarlagadda)

SENTRY-1773: Delta change cleaner should leave way more then a single entry intact (Alex Kolbasov, reviewed by Vamsee Yarlagadda)

Change-Id: I8f1f9911c7c445c3c6a9618619092ed7cfe543cb
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23455
Tested-by: Jenkins User
Reviewed-by: Alexander Kolbasov <ak...@cloudera.com>


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

Branch: refs/for/cdh5-1.5.1_ha
Commit: a4bd00113026066ced9fb483fde0874bd578a684
Parents: 5ac28f0
Author: Alexander Kolbasov <ak...@cloudera.com>
Authored: Mon Jun 5 14:01:08 2017 -0700
Committer: Alexander Kolbasov <ak...@cloudera.com>
Committed: Mon Jun 5 16:45:14 2017 -0700

----------------------------------------------------------------------
 .../provider/db/service/persistent/SentryStore.java     | 12 ++++++++----
 .../apache/sentry/service/thrift/ServiceConstants.java  |  8 ++++++++
 .../provider/db/service/persistent/TestSentryStore.java |  7 +++++--
 3 files changed, 21 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/a4bd0011/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index 891c1b1..d77c87a 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -499,23 +499,27 @@ public class SentryStore {
 
   /**
    * Purge delta change tables, {@link MSentryPermChange} and {@link MSentryPathChange}.
+   * The number of deltas to keep is configurable
    */
   public void purgeDeltaChangeTables() {
-    LOGGER.info("Purging MSentryPathUpdate and MSentyPermUpdate tables.");
+    final int changesToKeep = conf.getInt(ServerConfig.SENTRY_DELTA_KEEP_COUNT,
+            ServerConfig.SENTRY_DELTA_KEEP_COUNT_DEFAULT);
+    LOGGER.info("Purging MSentryPathUpdate and MSentyPermUpdate tables, leaving {} entries",
+            changesToKeep);
     try {
       tm.executeTransaction(new TransactionBlock<Object>() {
         @Override
         public Object execute(PersistenceManager pm) throws Exception {
           pm.setDetachAllOnCommit(false); // No need to detach objects
-          purgeDeltaChangeTableCore(MSentryPermChange.class, pm, 1);
+          purgeDeltaChangeTableCore(MSentryPermChange.class, pm, changesToKeep);
           LOGGER.info("MSentryPermChange table has been purged.");
-          purgeDeltaChangeTableCore(MSentryPathChange.class, pm, 1);
+          purgeDeltaChangeTableCore(MSentryPathChange.class, pm, changesToKeep);
           LOGGER.info("MSentryPathUpdate table has been purged.");
           return null;
         }
       });
     } catch (Exception e) {
-      LOGGER.error("Delta change cleaning process encounter an error.", e);
+      LOGGER.error("Delta change cleaning process encounter an error", e);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/a4bd0011/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 c14a854..9d11572 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
@@ -223,6 +223,14 @@ public class ServiceConstants {
     public static final String SENTRY_KERBEROS_TGT_AUTORENEW = "sentry.service.kerberos.tgt.autorenew";
     @Deprecated
     public static final Boolean SENTRY_KERBEROS_TGT_AUTORENEW_DEFAULT = false;
+
+    /**
+     * Number of path/priv deltas to keep around during cleaning
+     * The value which is too small may cause unnecessary full snapshots sent to the Name Node
+     * A value which is too large may cause slowdown due to too many deltas lying around in the DB.
+     */
+    public static final String SENTRY_DELTA_KEEP_COUNT = "sentry.server.delta.keep.count";
+    public static final int SENTRY_DELTA_KEEP_COUNT_DEFAULT = 200;
   }
 
   public static class ClientConfig {

http://git-wip-us.apache.org/repos/asf/sentry/blob/a4bd0011/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index 2225f23..7f9a661 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -64,6 +64,7 @@ import org.apache.sentry.provider.db.service.thrift.TSentryGrantOption;
 import org.apache.sentry.provider.db.service.thrift.TSentryGroup;
 import org.apache.sentry.provider.db.service.thrift.TSentryPrivilege;
 import org.apache.sentry.provider.file.PolicyFile;
+import org.apache.sentry.service.thrift.ServiceConstants;
 import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -2455,8 +2456,10 @@ public class TestSentryStore extends org.junit.Assert {
     assertEquals(0, sentryStore.getMSentryPathChanges().size());
 
     sentryStore.createSentryRole(role);
+    int privCleanCount = ServerConfig.SENTRY_DELTA_KEEP_COUNT_DEFAULT;
+    int extraPrivs = 5;
 
-    final int numPermChanges = 5;
+    final int numPermChanges = extraPrivs + privCleanCount;
     for (int i = 0; i < numPermChanges; i++) {
       TSentryPrivilege privilege = new TSentryPrivilege();
       privilege.setPrivilegeScope("Column");
@@ -2473,7 +2476,7 @@ public class TestSentryStore extends org.junit.Assert {
     assertEquals(numPermChanges, sentryStore.getMSentryPermChanges().size());
 
     sentryStore.purgeDeltaChangeTables();
-    assertEquals(1, sentryStore.getMSentryPermChanges().size());
+    assertEquals(privCleanCount, sentryStore.getMSentryPermChanges().size());
 
     // TODO: verify MSentryPathChange being purged.
     // assertEquals(1, sentryStore.getMSentryPathChanges().size());