You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by pr...@apache.org on 2014/05/21 22:26:20 UTC
git commit: SENTRY-178: Poor performance for Sentry Policy Service as
#of privileges is scaled up (Arun Suresh via Prasad Mujumdar)
Repository: incubator-sentry
Updated Branches:
refs/heads/master 5743455f9 -> afe8cd8df
SENTRY-178: Poor performance for Sentry Policy Service as #of privileges is scaled up (Arun Suresh via Prasad Mujumdar)
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/afe8cd8d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/afe8cd8d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/afe8cd8d
Branch: refs/heads/master
Commit: afe8cd8dfed8f0118a97c1d9377eef262717d9ea
Parents: 5743455
Author: Prasad Mujumdar <pr...@cloudera.com>
Authored: Wed May 21 13:26:06 2014 -0700
Committer: Prasad Mujumdar <pr...@cloudera.com>
Committed: Wed May 21 13:26:06 2014 -0700
----------------------------------------------------------------------
.../db/service/model/MSentryPrivilege.java | 5 +-
.../provider/db/service/model/MSentryRole.java | 3 +-
.../db/service/persistent/SentryStore.java | 4 +-
.../thrift/TestSentryServiceIntegration.java | 56 +++++++++++++++++---
4 files changed, 52 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/afe8cd8d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
index 4030205..82d701f 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
@@ -41,7 +41,7 @@ public class MSentryPrivilege {
private String URI;
private String action;
// roles this privilege is a part of
- private Set<MSentryRole> roles;
+ private final Set<MSentryRole> roles;
private long createTime;
private String grantorPrincipal;
@@ -135,8 +135,7 @@ public class MSentryPrivilege {
}
public void appendRole(MSentryRole role) {
- if (!roles.contains(role)) {
- roles.add(role);
+ if (roles.add(role)) {
role.appendPrivilege(this);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/afe8cd8d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
index e375a4c..86aaeb4 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java
@@ -99,8 +99,7 @@ public class MSentryRole {
}
public void appendPrivilege(MSentryPrivilege privilege) {
- if (!privileges.contains(privilege)) {
- privileges.add(privilege);
+ if (privileges.add(privilege)) {
privilege.appendRole(this);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/afe8cd8d/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 cd71a58..7971ea3 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
@@ -274,12 +274,10 @@ public class SentryStore {
if (mPrivilege == null) {
mPrivilege = convertToMSentryPrivilege(privilege);
}
- // Add privilege and role objects to each other. needed by datanucleus to model
- // m:n relationships correctly through a join table.
mPrivilege.appendRole(mRole);
- mRole.appendPrivilege(mPrivilege);
pm.makePersistent(mRole);
pm.makePersistent(mPrivilege);
+
CommitContext commit = commitUpdateTransaction(pm);
rollbackTransaction = false;
return commit;
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/afe8cd8d/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
index a2e877a..f51f518 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
@@ -19,6 +19,8 @@
package org.apache.sentry.provider.db.service.thrift;
import com.google.common.collect.Sets;
+
+import org.apache.sentry.core.common.ActiveRoleSet;
import org.apache.sentry.provider.db.service.persistent.SentryStore;
import org.apache.sentry.service.thrift.SentryServiceIntegrationBase;
import org.junit.Test;
@@ -54,17 +56,55 @@ public class TestSentryServiceIntegration extends SentryServiceIntegrationBase {
public void testGranRevokePrivilegeOnTableForRole() throws Exception {
String requestorUserName = ADMIN_USER;
Set<String> requestorUserGroupNames = Sets.newHashSet(ADMIN_GROUP);
- String roleName = "admin_r";
+ String roleName1 = "admin_r1";
+ String roleName2 = "admin_r2";
- client.dropRoleIfExists(requestorUserName, requestorUserGroupNames, roleName);
- client.createRole(requestorUserName, requestorUserGroupNames, roleName);
+ client.dropRoleIfExists(requestorUserName, requestorUserGroupNames, roleName1);
+ client.createRole(requestorUserName, requestorUserGroupNames, roleName1);
+
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table1", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table2", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table3", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table4", "ALL");
+
+
+ client.dropRoleIfExists(requestorUserName, requestorUserGroupNames, roleName2);
+ client.createRole(requestorUserName, requestorUserGroupNames, roleName2);
- client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName, "server", "db", "table", "ALL");
- Set<TSentryPrivilege> listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName);
- assertTrue("Privilege not assigned to role !!", listPrivilegesByRoleName.size() == 1);
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table1", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table2", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table3", "ALL");
+ client.grantTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table4", "ALL");
- client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName, "server", "db", "table", "ALL");
- listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName);
+ Set<TSentryPrivilege> listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName1);
+ assertEquals("Privilege not assigned to role1 !!", 4, listPrivilegesByRoleName.size());
+
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName2);
+ assertEquals("Privilege not assigned to role2 !!", 4, listPrivilegesByRoleName.size());
+
+
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table1", "ALL");
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName1);
+ assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 3);
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName2);
+ assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 4);
+
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table1", "ALL");
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName2);
+ assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 3);
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName1);
+ assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 3);
+
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table2", "ALL");
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table3", "ALL");
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName1, "server", "db", "table4", "ALL");
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName1);
+ assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 0);
+
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table2", "ALL");
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table3", "ALL");
+ client.revokeTablePrivilege(requestorUserName, requestorUserGroupNames, roleName2, "server", "db", "table4", "ALL");
+ listPrivilegesByRoleName = client.listPrivilegesByRoleName(requestorUserName, requestorUserGroupNames, roleName2);
assertTrue("Privilege not correctly revoked !!", listPrivilegesByRoleName.size() == 0);
}