sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sp...@apache.org
Subject sentry git commit: SENTRY-2046: Create a full snapshot if AUTHZ_PATHS_SNAPSHOT_ID is empty, even if HMS and Sentry Notifications are in sync (Arjun Mishra, reviewed by Sergio Pena, Na Li, kalyan kumar kalvagadda)
Date Tue, 21 Nov 2017 21:51:32 GMT
Repository: sentry
Updated Branches:
  refs/heads/master f3e93a475 -> 3dc8d2f90


SENTRY-2046: Create a full snapshot if AUTHZ_PATHS_SNAPSHOT_ID is empty, even if HMS and Sentry
Notifications are in sync (Arjun Mishra, reviewed by Sergio Pena, Na Li, kalyan kumar kalvagadda)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/3dc8d2f9
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/3dc8d2f9
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/3dc8d2f9

Branch: refs/heads/master
Commit: 3dc8d2f90e4336e736dbe1a73bb70ad1c77f95c1
Parents: f3e93a4
Author: Sergio Pena <sergio.pena@cloudera.com>
Authored: Tue Nov 21 15:50:43 2017 -0600
Committer: Sergio Pena <sergio.pena@cloudera.com>
Committed: Tue Nov 21 15:50:43 2017 -0600

----------------------------------------------------------------------
 .../db/service/persistent/SentryStore.java      | 17 ++++
 .../sentry/service/thrift/HMSFollower.java      | 11 ++-
 .../sentry/service/thrift/TestHMSFollower.java  | 95 ++++++++++++++++++++
 3 files changed, 122 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/3dc8d2f9/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 4dc2bf6..f32a745 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
@@ -3162,6 +3162,23 @@ public class SentryStore {
   }
 
   /**
+   * Tells if there are any records in MAuthzPathsSnapshotId
+   *
+   * @return true if there are no entries in <code>MAuthzPathsSnapshotId</code>
+   * false if there are entries
+   * @throws Exception
+   */
+  public boolean isAuthzPathsSnapshotEmpty() 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, MAuthzPathsSnapshotId.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.

http://git-wip-us.apache.org/repos/asf/sentry/blob/3dc8d2f9/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 c4cc918..c1471d1 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
@@ -235,9 +235,11 @@ public class HMSFollower implements Runnable, AutoCloseable, PubSub.Subscriber
{
   /**
    * Checks if a new full HMS snapshot request is needed by checking if:
    * <ul>
-   *   <li>No snapshots has been persisted yet.</li>
+   *   <li>Sentry HMS Notification table is EMPTY</li>
+   *   <li>HDFSSync is enabled and Sentry Authz Snapshot table is EMPTY</li>
    *   <li>The current notification Id on the HMS is less than the
    *   latest processed by Sentry.</li>
+   *   <li>Full Snapshot Signal is detected</li>
    * </ul>
    *
    * @param latestSentryNotificationId The notification Id to check against the HMS
@@ -249,6 +251,13 @@ public class HMSFollower implements Runnable, AutoCloseable, PubSub.Subscriber
{
       return true;
     }
 
+    // Once HDFS sync is enabled, and if MAuthzPathsSnapshotId
+    // table is still empty, we need to request a full snapshot
+    if(hdfsSyncEnabled && sentryStore.isAuthzPathsSnapshotEmpty()) {
+      LOGGER.debug("HDFSSync is enabled and MAuthzPathsSnapshotId table is empty. Need to
request a full snapshot");
+      return true;
+    }
+
     long currentHmsNotificationId = notificationFetcher.getCurrentNotificationId();
     if (currentHmsNotificationId < latestSentryNotificationId) {
       LOGGER.info("The latest notification ID on HMS is less than the latest notification
ID "

http://git-wip-us.apache.org/repos/asf/sentry/blob/3dc8d2f9/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
index bbcf093..5dd9b1b 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
@@ -239,6 +239,101 @@ public class TestHMSFollower {
   }
 
   @Test
+  public void testPersistAFullSnapshotWhenAuthzsnapshotIsEmptyAndHDFSSyncIsEnabled() throws
Exception {
+    /*
+     * TEST CASE
+     *
+     * Simulates (by using mocks) the following:
+     *
+     * Disable HDFSSync before triggering a full snapshot
+     *
+     * HMS client always returns the paths image with the eventId == 1.
+     *
+     * On the 1st run:  Sentry notification table is empty, so this
+     * should trigger a new full HMS snapshot request with the eventId = 1
+     * but it should not persist it, in stead only set last
+     * last processed notification Id. This will prevent a
+     * unless until notifications are out of sync or hdfs sync is enabled
+     *
+     * On the 2nd run: Just enable hdfs sync and a full snapshot should be triggered
+     * because MAuthzPathsSnapshotId table is empty
+     *
+     */
+
+    configuration.set(ServiceConstants.ServerConfig.PROCESSOR_FACTORIES, "");
+    configuration.set(ServiceConstants.ServerConfig.SENTRY_POLICY_STORE_PLUGINS, "");
+
+    final long SENTRY_PROCESSED_EVENT_ID = SentryStore.EMPTY_NOTIFICATION_ID;
+    final long HMS_PROCESSED_EVENT_ID = 1L;
+
+    // Mock that returns a full snapshot
+    Map<String, Collection<String>> snapshotObjects = new HashMap<>();
+    snapshotObjects.put("db", Sets.newHashSet("/db"));
+    snapshotObjects.put("db.table", Sets.newHashSet("/db/table"));
+    PathsImage fullSnapshot = new PathsImage(snapshotObjects, HMS_PROCESSED_EVENT_ID, 1);
+
+    SentryHMSClient sentryHmsClient = Mockito.mock(SentryHMSClient.class);
+    when(sentryHmsClient.getFullSnapshot()).thenReturn(fullSnapshot);
+
+    HMSFollower hmsFollower = new HMSFollower(configuration, sentryStore, null,
+        hmsConnectionMock, hiveInstance);
+    hmsFollower.setSentryHmsClient(sentryHmsClient);
+
+    // 1st run should get a full snapshot because hms notificaions is empty
+    // but it should never be persisted because HDFS sync is disabled
+    when(sentryStore.getLastProcessedNotificationID()).thenReturn(SENTRY_PROCESSED_EVENT_ID);
+    when(sentryStore.isHmsNotificationEmpty()).thenReturn(true);
+    hmsFollower.run();
+    verify(sentryStore, times(0)).persistFullPathsImage(
+        fullSnapshot.getPathImage(), fullSnapshot.getId());
+    // Since hdfs sync is disabled we would set last processed notifications
+    // and since we did trigger createFullSnapshot() method we won't process any notifications
+    verify(sentryStore, times(1)).setLastProcessedNotificationID(fullSnapshot.getId());
+    verify(sentryStore, times(0)).persistLastProcessedNotificationID(fullSnapshot.getId());
+
+    reset(sentryStore);
+
+    //Re-enable HDFS Sync and simply start the HMS follower thread, full snap shot
+    // should be triggered because MAuthzPathsSnapshotId table is empty
+    configuration.set(ServiceConstants.ServerConfig.PROCESSOR_FACTORIES, "org.apache.sentry.hdfs.SentryHDFSServiceProcessorFactory");
+    configuration.set(ServiceConstants.ServerConfig.SENTRY_POLICY_STORE_PLUGINS, "org.apache.sentry.hdfs.SentryPlugin");
+
+    //Create a new hmsFollower instance since configuration is changing
+    hmsFollower = new HMSFollower(configuration, sentryStore, null,
+        hmsConnectionMock, hiveInstance);
+    hmsFollower.setSentryHmsClient(sentryHmsClient);
+
+
+    //Set last processed notification Id to match the full new value 1L
+    final long LATEST_EVENT_ID = 1L;
+    when(sentryStore.getLastProcessedNotificationID()).thenReturn(LATEST_EVENT_ID);
+    //Mock that sets isHmsNotificationEmpty to false
+    when(sentryStore.isHmsNotificationEmpty()).thenReturn(false);
+    // Mock that sets the current HMS notification ID. Set it to match
+    // last processed notification Id so that doesn't trigger a full snapshot
+    when(hmsClientMock.getCurrentNotificationEventId())
+        .thenReturn(new CurrentNotificationEventId(LATEST_EVENT_ID));
+    //Mock that sets getting next notification eve
+    when(hmsClientMock.getNextNotification(Mockito.eq(HMS_PROCESSED_EVENT_ID - 1), Mockito.eq(Integer.MAX_VALUE),
+        (NotificationFilter) Mockito.notNull()))
+        .thenReturn(new NotificationEventResponse(
+            Arrays.<NotificationEvent>asList(
+                new NotificationEvent(LATEST_EVENT_ID, 0, "", "")
+            )
+        ));
+    //Mock that sets isAuthzPathsSnapshotEmpty to true so trigger this particular test
+    when(sentryStore.isAuthzPathsSnapshotEmpty()).thenReturn(true);
+
+    hmsFollower.run();
+    verify(sentryStore, times(1)).persistFullPathsImage(
+        fullSnapshot.getPathImage(), fullSnapshot.getId());
+    verify(sentryStore, times(0)).setLastProcessedNotificationID(fullSnapshot.getId());
+    verify(sentryStore, times(0)).persistLastProcessedNotificationID(fullSnapshot.getId());
+
+    reset(sentryStore);
+  }
+
+  @Test
   public void testPersistAFullSnapshotWhenLastHmsNotificationIsLowerThanLastProcessed()
       throws Exception {
     /*


Mime
View raw message