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 <[email protected]> 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 <[email protected]> Authored: Mon Jun 5 14:01:08 2017 -0700 Committer: Alexander Kolbasov <[email protected]> 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());
