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

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

Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign 445eeae67 -> 4a1365de9


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


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

Branch: refs/heads/sentry-ha-redesign
Commit: 4a1365de92281f87792a75a4829a5a3292446dda
Parents: 445eeae
Author: Alexander Kolbasov <ak...@cloudera.com>
Authored: Mon Jun 5 13:59:18 2017 -0700
Committer: Alexander Kolbasov <ak...@cloudera.com>
Committed: Mon Jun 5 13:59:18 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/4a1365de/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 97b3636..37eb0b2 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
@@ -517,23 +517,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/4a1365de/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 638c079..099ebd0 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/4a1365de/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 c5dddfb..23b685b 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
@@ -59,6 +59,7 @@ import org.apache.sentry.provider.db.service.thrift.TSentryGroup;
 import org.apache.sentry.provider.db.service.thrift.TSentryPrivilege;
 import org.apache.sentry.provider.db.service.thrift.TSentryRole;
 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;
@@ -2902,8 +2903,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");
@@ -2920,7 +2923,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());