sentry-commits mailing list archives

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.li@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;