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/05/27 00:20:50 UTC
sentry git commit: 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)
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 <ak...@cloudera.com>
Authored: Fri May 26 17:20:28 2017 -0700
Committer: Alexander Kolbasov <ak...@cloudera.com>
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);
+ }
}