sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vam...@apache.org
Subject sentry git commit: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB (Vamsee Yarlagadda, reviewed by Alexander Kolbasov)
Date Fri, 09 Dec 2016 01:54:07 GMT
Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign 7ed5c4864 -> 7c3456020


SENTRY-1557: getRolesForGroups(),getRolesForUsers() does too many trips to the the DB (Vamsee
Yarlagadda, reviewed by Alexander Kolbasov)


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

Branch: refs/heads/sentry-ha-redesign
Commit: 7c345602033dcad6f6470d2b990aeeaf49fd630e
Parents: 7ed5c48
Author: Vamsee Yarlagadda <vamsee@cloudera.com>
Authored: Wed Dec 7 23:42:36 2016 -0800
Committer: Vamsee Yarlagadda <vamsee@cloudera.com>
Committed: Thu Dec 8 17:49:12 2016 -0800

----------------------------------------------------------------------
 .../db/service/persistent/SentryStore.java      | 20 ++--
 .../db/service/persistent/TestSentryStore.java  | 96 ++++++++++++++++++++
 2 files changed, 104 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/7c345602/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 5218715..6ba040d 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
@@ -1255,12 +1255,10 @@ public class SentryStore {
     Set<MSentryRole> result = Sets.newHashSet();
     if (groups != null) {
       Query query = pm.newQuery(MSentryGroup.class);
-      query.setFilter("this.groupName == t");
-      query.declareParameters("java.lang.String t");
-      query.setUnique(true);
-      for (String group : groups) {
-        MSentryGroup sentryGroup = (MSentryGroup) query.execute(group.trim());
-        if (sentryGroup != null) {
+      query.setFilter(":p1.contains(this.groupName)");
+      List<MSentryGroup> sentryGroups = (List) query.execute(groups.toArray());
+      if (sentryGroups != null) {
+        for (MSentryGroup sentryGroup : sentryGroups) {
           result.addAll(sentryGroup.getRoles());
         }
       }
@@ -1272,12 +1270,10 @@ public class SentryStore {
     Set<MSentryRole> result = Sets.newHashSet();
     if (users != null) {
       Query query = pm.newQuery(MSentryUser.class);
-      query.setFilter("this.userName == t");
-      query.declareParameters("java.lang.String t");
-      query.setUnique(true);
-      for (String user : users) {
-        MSentryUser sentryUser = (MSentryUser) query.execute(user.trim());
-        if (sentryUser != null) {
+      query.setFilter(":p1.contains(this.userName)");
+      List<MSentryUser> sentryUsers = (List) query.execute(users.toArray());
+      if (sentryUsers != null) {
+        for (MSentryUser sentryUser : sentryUsers) {
           result.addAll(sentryUser.getRoles());
         }
       }

http://git-wip-us.apache.org/repos/asf/sentry/blob/7c345602/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index ae9f1ed..14b5e8e 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -381,6 +381,102 @@ public class TestSentryStore extends org.junit.Assert {
   }
 
   @Test
+  public void testGetTSentryRolesForUsers() throws Exception {
+    // Test the method getTSentryRolesByUserNames according to the following test data:
+    // user1->r1
+    // user2->r3
+    // user3->r2
+    // user4->r3, r2
+    String roleName1 = "r1";
+    String roleName2 = "r2";
+    String roleName3 = "r3";
+    String user1 = "u1";
+    String user2 = "u2";
+    String user3 = "u3";
+    String user4 = "u4";
+
+    createRole(roleName1);
+    createRole(roleName2);
+    createRole(roleName3);
+    sentryStore.alterSentryRoleAddUsers(roleName1, Sets.newHashSet(user1));
+    sentryStore.alterSentryRoleAddUsers(roleName2, Sets.newHashSet(user3));
+    sentryStore.alterSentryRoleAddUsers(roleName2, Sets.newHashSet(user4));
+    sentryStore.alterSentryRoleAddUsers(roleName3, Sets.newHashSet(user2, user4));
+
+    Set<String> userSet1 = Sets.newHashSet(user1, user2, user3);
+    Set<String> roleSet1 = Sets.newHashSet(roleName1, roleName2, roleName3);
+
+    Set<String> userSet2 = Sets.newHashSet(user4);
+    Set<String> roleSet2 = Sets.newHashSet(roleName2, roleName3);
+
+    Set<String> userSet3 = Sets.newHashSet("foo");
+    Set<String> roleSet3 = Sets.newHashSet();
+
+    // Query for multiple users
+    Set<String> roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet1));
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet1).size());
+
+    // Query for single users
+    roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet2));
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet2).size());
+
+    // Query for non-existing user
+    roles = convertToRoleNameSet(sentryStore.getTSentryRolesByUserNames(userSet3));
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet3).size());
+  }
+
+  private Set<String> convertToRoleNameSet(Set<TSentryRole> tSentryRoles) {
+    Set<String> roleNameSet = Sets.newHashSet();
+    for (TSentryRole role : tSentryRoles) {
+      roleNameSet.add(role.getRoleName());
+    }
+    return roleNameSet;
+  }
+
+  @Test
+  public void testGetTSentryRolesForGroups() throws Exception {
+    // Test the method getRoleNamesForGroups according to the following test data:
+    // group1->r1
+    // group2->r2
+    // group3->r2
+    String grantor = "g1";
+    String roleName1 = "r1";
+    String roleName2 = "r2";
+    String roleName3 = "r3";
+    String group1 = "group1";
+    String group2 = "group2";
+    String group3 = "group3";
+
+    createRole(roleName1);
+    createRole(roleName2);
+    createRole(roleName3);
+    sentryStore.alterSentryRoleAddGroups(grantor, roleName1, Sets.newHashSet(new TSentryGroup(group1)));
+    sentryStore.alterSentryRoleAddGroups(grantor, roleName2, Sets.newHashSet(new TSentryGroup(group2),
+        new TSentryGroup(group3)));
+
+    Set<String> groupSet1 = Sets.newHashSet(group1, group2, group3);
+    Set<String> roleSet1 = Sets.newHashSet(roleName1, roleName2);
+
+    Set<String> groupSet2 = Sets.newHashSet(group1);
+    Set<String> roleSet2 = Sets.newHashSet(roleName1);
+
+    Set<String> groupSet3 = Sets.newHashSet("foo");
+    Set<String> roleSet3 = Sets.newHashSet();
+
+    // Query for multiple groups
+    Set<String> roles = sentryStore.getRoleNamesForGroups(groupSet1);
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet1).size());
+
+    // Query for single group
+    roles = sentryStore.getRoleNamesForGroups(groupSet2);
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet2).size());
+
+    // Query for non-existing group
+    roles = sentryStore.getRoleNamesForGroups(groupSet3);
+    assertEquals("Returned roles should match the expected roles", 0, Sets.symmetricDifference(roles,
roleSet3).size());
+  }
+
+  @Test
   public void testGrantRevokePrivilege() throws Exception {
     String roleName = "test-privilege";
     String grantor = "g1";


Mime
View raw message