SENTRY-1762: notification id's in SENTRY_HMS_NOTIFICATION_ID should be purged periodically (kalyan kumar kalvagadda, Reviewed by Na Li, Alexander Kolbasov, Sergio Pena)
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/d4b64faa Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/d4b64faa Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/d4b64faa Branch: refs/heads/master Commit: d4b64faa307aa17776ac6b5011f0a580b12b8074 Parents: 2fcd69a Author: Kalyan Kumar Kalvagadda <[email protected]> Authored: Wed Jul 12 13:53:23 2017 -0500 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Wed Jul 12 13:53:23 2017 -0500 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 82 +++++++++++++++++++- .../sentry/service/thrift/SentryService.java | 1 + .../sentry/service/thrift/ServiceConstants.java | 6 ++ .../db/service/persistent/TestSentryStore.java | 36 ++++++++- 4 files changed, 123 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/d4b64faa/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 350c922..979e45b 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 @@ -589,6 +589,28 @@ public class SentryStore { } /** + * Purge notification id table, keeping a specified number of entries. + * @param pm a {@link PersistenceManager} instance. + * @param changesToKeep the number of changes the caller want to keep. + */ + @VisibleForTesting + protected void purgeNotificationIdTableCore(PersistenceManager pm, + long changesToKeep) { + Preconditions.checkArgument(changesToKeep > 0, + "You need to keep at least one entry in SENTRY_HMS_NOTIFICATION_ID table"); + long lastNotificationID = getLastProcessedNotificationIDCore(pm); + Query query = pm.newQuery(MSentryHmsNotification.class); + + // It is an approximation of "SELECT ... LIMIT CHANGE_TO_KEEP" in SQL, because JDO w/ derby + // does not support "LIMIT". + // See: http://www.datanucleus.org/products/datanucleus/jdo/jdoql_declarative.html + query.setFilter("notificationId <= maxNotificationIdDeleted"); + query.declareParameters("long maxNotificationIdDeleted"); + long numDeleted = query.deletePersistentAll(lastNotificationID - changesToKeep); + LOGGER.info("Purged {} of {}", numDeleted, MSentryHmsNotification.class.getSimpleName()); + } + + /** * Purge delta change tables, {@link MSentryPermChange} and {@link MSentryPathChange}. * The number of deltas to keep is configurable */ @@ -610,11 +632,34 @@ public class SentryStore { } }); } catch (Exception e) { - LOGGER.error("Delta change cleaning process encounter an error", e); + LOGGER.error("Delta change cleaning process encountered an error", e); } } /** + * Purge hms notification id table , {@link MSentryHmsNotification}. + * The number of notifications id's to be kept is based on configuration + * sentry.server.delta.keep.count + */ + public void purgeNotificationIdTable() { + final int changesToKeep = conf.getInt(ServerConfig.SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT, + ServerConfig.SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT); + LOGGER.debug("Purging MSentryHmsNotification table, 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 + purgeNotificationIdTableCore(pm, changesToKeep); + return null; + } + }); + } catch (Exception e) { + LOGGER.error("MSentryHmsNotification cleaning process encountered an error", e); + } + } + /** * Alter a given sentry role to grant a privilege. * * @param grantorPrincipal User name @@ -2950,6 +2995,24 @@ public class SentryStore { } /** + * Tells if there are any records in MSentryHmsNotification + * + * @return true if there are no entries in {@link MSentryHmsNotification} + * false if there are entries + * @throws Exception + */ + @VisibleForTesting + boolean isNotificationIDTableEmpty() throws Exception { + return tm.executeTransactionWithRetry( + new TransactionBlock<Boolean>() { + public Boolean execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects + return isTableEmptyCore(pm, MSentryHmsNotification.class); + } + }); + } + + /** * Updates authzObj -> [Paths] mapping to replace an existing path with a new one * given an authzObj. As well as persist the corresponding delta path change to * MSentryPathChange table in a single transaction. @@ -3801,6 +3864,23 @@ public class SentryStore { } /** + * Fetch all {@link MSentryHmsNotification} in the database. It should only be used in the tests. + * + * @return a list of notifications ids. + * @throws Exception + */ + @VisibleForTesting + List<MSentryHmsNotification> getMSentryHmsNotificationCore() throws Exception { + return tm.executeTransaction( + new TransactionBlock<List<MSentryHmsNotification>>() { + public List<MSentryHmsNotification> execute(PersistenceManager pm) throws Exception { + Query query = pm.newQuery(MSentryHmsNotification.class); + return (List<MSentryHmsNotification>) query.execute(); + } + }); + } + + /** * Checks if any MSentryChange object exists with the given changeID. * * @param pm PersistenceManager http://git-wip-us.apache.org/repos/asf/sentry/blob/d4b64faa/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java index 322197b..8b1d8fc 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java @@ -381,6 +381,7 @@ public class SentryService implements Callable, SigUtils.SigListener { public void run() { if (leaderMonitor.isLeader()) { sentryStore.purgeDeltaChangeTables(); + sentryStore.purgeNotificationIdTable(); } } }; http://git-wip-us.apache.org/repos/asf/sentry/blob/d4b64faa/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 193c611..d85e7b4 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 @@ -232,6 +232,12 @@ public class ServiceConstants { */ public static final String SENTRY_DELTA_KEEP_COUNT = "sentry.server.delta.keep.count"; public static final int SENTRY_DELTA_KEEP_COUNT_DEFAULT = 200; + + /** + * Number of notification id's to keep around during cleaning + */ + public static final String SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT = "sentry.server.delta.keep.count"; + public static final int SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT = 100; } public static class ClientConfig { http://git-wip-us.apache.org/repos/asf/sentry/blob/d4b64faa/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 395cba6..51f6c5d 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,7 +59,6 @@ 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; @@ -3074,6 +3073,41 @@ public class TestSentryStore extends org.junit.Assert { // assertEquals(1, sentryStore.getMSentryPathChanges().size()); } + @Test + public void testpurgeNotificationIdTable() throws Exception { + + int totalentires = 200; + int remainingEntires = ServerConfig.SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT; + assertTrue(sentryStore.isNotificationIDTableEmpty()); + for(int id = 1; id <= totalentires; id++) { + sentryStore.persistLastProcessedNotificationID((long)id); + } + assertEquals(totalentires, sentryStore.getMSentryHmsNotificationCore().size()); + sentryStore.purgeNotificationIdTable(); + + // Make sure that sentry store still hold entries based on SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT + assertEquals(remainingEntires, sentryStore.getMSentryHmsNotificationCore().size()); + + sentryStore.purgeNotificationIdTable(); + // Make sure that sentry store still hold entries based on SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT + assertEquals(remainingEntires, sentryStore.getMSentryHmsNotificationCore().size()); + + sentryStore.clearAllTables(); + + + totalentires = 50; + for(int id = 1; id <= totalentires; id++) { + sentryStore.persistLastProcessedNotificationID((long)id); + } + assertEquals(totalentires, sentryStore.getMSentryHmsNotificationCore().size()); + sentryStore.purgeNotificationIdTable(); + + // Make sure that sentry store still holds all the entries as total entries is less than + // SENTRY_HMS_NOTIFICATION_ID_KEEP_COUNT_DEFAULT. + assertEquals(totalentires, sentryStore.getMSentryHmsNotificationCore().size()); + } + + /** * This test verifies that in the case of concurrently updating delta change tables, no gap * between change ID was made. All the change IDs must be consecutive ({@see SENTRY-1643}).
