Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign 9c11c32c6 -> cc923c8db
SENTRY-1774: HMSFollower should always depend on persisted information to decide is full snapshot is needed (Kalyan Kalvagadda, reviewed by Vamsee Yarlagadda, Sergio Pena and Alex Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/cc923c8d Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/cc923c8d Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/cc923c8d Branch: refs/heads/sentry-ha-redesign Commit: cc923c8db0d7d909148e47f7290d2e5217d88f90 Parents: 9c11c32 Author: Alexander Kolbasov <[email protected]> Authored: Fri May 26 17:20:28 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Fri May 26 17:20:28 2017 -0700 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 36 ++++++++++++++++++++ .../sentry/service/thrift/HMSFollower.java | 19 ++--------- .../db/service/persistent/TestSentryStore.java | 23 +++++++++++++ 3 files changed, 62 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/cc923c8d/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 f26894a..cb05a84 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 @@ -2798,6 +2798,23 @@ public class SentryStore { } /** + * Tells if there are any records in MAuthzPathsMapping + * + * @return true if there are no entries in <code>MAuthzPathsMapping</code> + * false if there are entries + * @throws Exception + */ + public boolean isAuthzPathsMappingEmpty() 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, MAuthzPathsMapping.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. @@ -2862,6 +2879,25 @@ public class SentryStore { return (MAuthzPathsMapping) query.execute(authzObj); } + /** + * Checks if the table associated with class provided is empty + * + * @param pm PersistenceManager + * @param clazz class + * @return True is the table is empty + * False if it not. + */ + private boolean isTableEmptyCore(PersistenceManager pm, Class clazz) { + Query query = pm.newQuery(clazz); + // setRange is implemented efficiently for MySQL, Postgresql (using the LIMIT SQL keyword) + // and Oracle (using the ROWNUM keyword), with the query only finding the objects required + // by the user directly in the datastore. For other RDBMS the query will retrieve all + // objects up to the "to" record, and will not pass any unnecessary objects that are before + // the "from" record. + query.setRange(0, 1); + return ((List<MAuthzPathsMapping>) query.execute()).isEmpty(); + } + @VisibleForTesting List<MPath> getMPaths() throws Exception { return tm.executeTransaction(new TransactionBlock<List<MPath>>() { http://git-wip-us.apache.org/repos/asf/sentry/blob/cc923c8d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java index 58e8881..d410a6c 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java @@ -82,28 +82,14 @@ public class HMSFollower implements Runnable, AutoCloseable { private final SentryStore sentryStore; private String hiveInstance; - private boolean needHiveSnapshot = true; private boolean needLogHMSSupportReady = true; private final LeaderStatusMonitor leaderMonitor; HMSFollower(Configuration conf, SentryStore store, LeaderStatusMonitor leaderMonitor) { LOGGER.info("HMSFollower is being initialized"); - Long lastProcessedNotificationID; authzConf = conf; this.leaderMonitor = leaderMonitor; sentryStore = store; - - try { - // Initializing lastProcessedNotificationID based on the latest persisted notification ID. - lastProcessedNotificationID = sentryStore.getLastProcessedNotificationID(); - } catch (Exception e) { - LOGGER.error("Failed to get the last processed notification id from sentry store, " + - "Skipping the processing", e); - needHiveSnapshot = true; - return; - } - // If lastProcessedNotificationID is empty, need to retrieve a full hive snapshot, - needHiveSnapshot = (lastProcessedNotificationID == SentryStore.EMPTY_CHANGE_ID); } @VisibleForTesting @@ -267,7 +253,9 @@ public class HMSFollower implements Runnable, AutoCloseable { } try { - if (needHiveSnapshot) { + // Decision of taking full snapshot is based on AuthzPathsMapping information persisted + // in the sentry persistent store. If AuthzPathsMapping is empty, shapshot is needed. + if (sentryStore.isAuthzPathsMappingEmpty()) { // TODO: expose time used for full update in the metrics // To ensure point-in-time snapshot consistency, need to make sure @@ -311,7 +299,6 @@ public class HMSFollower implements Runnable, AutoCloseable { // lastProcessedNotificationID instead of getting it from persistent store. lastProcessedNotificationID = eventIDAfter.getEventId(); sentryStore.persistFullPathsImage(pathsFullSnapshot); - needHiveSnapshot = false; sentryStore.persistLastProcessedNotificationID(eventIDAfter.getEventId()); // Wake up any HMS waiters that could have been put on hold before getting the eventIDBefore value. wakeUpWaitingClientsForSync(lastProcessedNotificationID); http://git-wip-us.apache.org/repos/asf/sentry/blob/cc923c8d/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 440b0e9..c5dddfb 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 @@ -3033,4 +3033,27 @@ public class TestSentryStore extends org.junit.Assert { } } } + + @Test + public void testIsAuthzPathsMappingEmpty() throws Exception { + // Add "db1.table1" authzObj + Long lastNotificationId = sentryStore.getLastProcessedNotificationID(); + PathsUpdate addUpdate = new PathsUpdate(1, false); + addUpdate.newPathChange("db1.table"). + addToAddPaths(Arrays.asList("db1", "tbl1")); + addUpdate.newPathChange("db1.table"). + addToAddPaths(Arrays.asList("db1", "tbl2")); + + assertEquals(sentryStore.isAuthzPathsMappingEmpty(), true); + sentryStore.addAuthzPathsMapping("db1.table", + Sets.newHashSet("db1/tbl1", "db1/tbl2"), addUpdate); + PathsImage pathsImage = sentryStore.retrieveFullPathsImage(); + Map<String, Set<String>> pathImage = pathsImage.getPathImage(); + assertEquals(1, pathImage.size()); + assertEquals(2, pathImage.get("db1.table").size()); + assertEquals(2, sentryStore.getMPaths().size()); + assertEquals(sentryStore.isAuthzPathsMappingEmpty(), false); + sentryStore.clearAllTables(); + assertEquals(sentryStore.isAuthzPathsMappingEmpty(), true); + } }
