sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject 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)
Date Sat, 27 May 2017 00:20:50 GMT
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 <akolb@cloudera.com>
Authored: Fri May 26 17:20:28 2017 -0700
Committer: Alexander Kolbasov <akolb@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);
+  }
 }


Mime
View raw message