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 <[email protected]> Authored: Tue Nov 21 15:50:43 2017 -0600 Committer: Sergio Pena <[email protected]> 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 { /*
