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)
Change-Id: I2e136ea96f0bb36b4beb0845e2fef0f5b9700ddc Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23138 Tested-by: Jenkins User Reviewed-by: Alexander Kolbasov <[email protected]> Reviewed-by: Na Li <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/926c6d08 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/926c6d08 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/926c6d08 Branch: refs/for/cdh5-1.5.1_ha Commit: 926c6d08d71cac0d67cedc512b98ade00bc08fbe Parents: c1dbc2c Author: Alexander Kolbasov <[email protected]> Authored: Fri May 26 17:20:28 2017 -0700 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Tue May 30 08:39:16 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/926c6d08/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 059cf20..2a96811 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 @@ -2650,6 +2650,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. @@ -2714,6 +2731,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/926c6d08/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 84fff75..2932055 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/926c6d08/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 dc48dd9..2225f23 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 @@ -2586,4 +2586,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); + } }
