You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by co...@apache.org on 2017/11/22 15:50:06 UTC
[4/9] 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)
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/akolb-cli
Commit: 3dc8d2f90e4336e736dbe1a73bb70ad1c77f95c1
Parents: f3e93a4
Author: Sergio Pena <se...@cloudera.com>
Authored: Tue Nov 21 15:50:43 2017 -0600
Committer: Sergio Pena <se...@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 {
/*