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 <[email protected]> Authored: Wed May 21 13:26:06 2014 -0700 Committer: Prasad Mujumdar <[email protected]> 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); }
