This is an automated email from the ASF dual-hosted git repository. linaataustin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sentry.git
The following commit(s) were added to refs/heads/master by this push: new ae2e91f SENTRY-2422: HMS synchronization is causing multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID (Na Li, reviewed by Kalyan Kumar Kalvagadda) ae2e91f is described below commit ae2e91fd2e06ba6c2fedd5e5d6efdea164daf176 Author: lina.li <lina...@cloudera.com> AuthorDate: Sat Jul 25 14:52:31 2020 -0500 SENTRY-2422: HMS synchronization is causing multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID (Na Li, reviewed by Kalyan Kumar Kalvagadda) --- .../db/service/persistent/SentryStore.java | 21 +++++++++++++--- .../db/service/persistent/TestSentryStore.java | 29 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index d2a46cb..63e5dfe 100644 --- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -3341,7 +3341,7 @@ public class SentryStore implements SentryStoreInterface { pm.setDetachAllOnCommit(false); // No need to detach objects deleteNotificationsSince(pm, notificationID + 1); // persist the notification ID - pm.makePersistent(new MSentryHmsNotification(notificationID)); + persistUniqueNotificationIDCore(pm, notificationID); // persist the full snapshot long snapshotID = getCurrentAuthzPathsSnapshotID(pm); @@ -4470,6 +4470,21 @@ public class SentryStore implements SentryStoreInterface { } /** + * Make sure the persisted notification ID is unique. There is no entries of duplicated value + * @param pm + * @param notificationId + * @return + */ + MSentryHmsNotification persistUniqueNotificationIDCore(PersistenceManager pm, Long notificationId) { + Long maxNotificationId = getMaxPersistedIDCore(pm, MSentryHmsNotification.class, "notificationId", EMPTY_NOTIFICATION_ID); + if (notificationId <= maxNotificationId) { + return new MSentryHmsNotification(maxNotificationId); + } + + return pm.makePersistent(new MSentryHmsNotification(notificationId)); + } + + /** * Set the notification ID of last processed HMS notification and remove all * subsequent notifications stored. */ @@ -4478,7 +4493,7 @@ public class SentryStore implements SentryStoreInterface { tm.executeTransaction( pm -> { deleteNotificationsSince(pm, notificationId + 1); - return pm.makePersistent(new MSentryHmsNotification(notificationId)); + return persistUniqueNotificationIDCore(pm, notificationId); }); } @@ -4488,7 +4503,7 @@ public class SentryStore implements SentryStoreInterface { public void persistLastProcessedNotificationID(final Long notificationId) throws Exception { LOGGER.debug("Persisting Last Processed Notification ID {}", notificationId); tm.executeTransaction( - pm -> pm.makePersistent(new MSentryHmsNotification(notificationId))); + pm -> persistUniqueNotificationIDCore(pm, notificationId)); } /** diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index fd14963..f538988 100644 --- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -18,6 +18,7 @@ package org.apache.sentry.provider.db.service.persistent; +import com.codahale.metrics.Gauge; import java.io.File; import java.time.Instant; import java.util.ArrayList; @@ -2825,6 +2826,34 @@ public class TestSentryStore extends org.junit.Assert { } @Test + public void testPersistOnlyUniqueNotificationId() throws Exception { + long notificationID = 1; + + // Persist the same ID twice should not cause any issues + sentryStore.persistLastProcessedNotificationID(notificationID); + sentryStore.persistLastProcessedNotificationID(notificationID); + + // Retrieving number of entries, and only unique values are stored + Gauge<Long> notificationGage = sentryStore.getLastNotificationIdGauge(); + assertEquals((long)1, notificationGage.getValue().longValue()); + } + + @Test + public void testPersistOnlyMaxUniqueNotificationId() throws Exception { + long notificationID = 1; + + // Persist the larger ID first, then less value + sentryStore.persistLastProcessedNotificationID(notificationID + 2); + sentryStore.persistLastProcessedNotificationID(notificationID); + sentryStore.persistLastProcessedNotificationID(notificationID + 1); + + // Retrieving latest peristed ID should match with the max persisted ID + long latestID = sentryStore.getLastProcessedNotificationID(); + assertEquals(notificationID + 2, latestID); + } + + + @Test public void testAddDeleteAuthzPathsMapping() throws Exception { long notificationID = 0;