sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vam...@apache.org
Subject [38/52] [abbrv] 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 Wed, 14 Jun 2017 00:57:16 GMT
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 <akolb@cloudera.com>
Reviewed-by: Na Li <lina.li@cloudera.com>


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 <akolb@cloudera.com>
Authored: Fri May 26 17:20:28 2017 -0700
Committer: Kalyan Kumar Kalvagadda <kkalyan@cloudera.com>
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);
+  }
 }


Mime
View raw message