You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ka...@apache.org on 2018/12/14 16:35:22 UTC
sentry git commit: SENTRY-2249: Enable batch insert of HMS paths in
Full Snapshot. (Kalyan Kumar Kalvagadda reviewed by Sergio Pena,
Na Li and Arjun Mishra)
Repository: sentry
Updated Branches:
refs/heads/master 41b090fbe -> d7fe39869
SENTRY-2249: Enable batch insert of HMS paths in Full Snapshot. (Kalyan Kumar Kalvagadda reviewed by Sergio Pena, Na Li and Arjun Mishra)
Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/d7fe3986
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/d7fe3986
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/d7fe3986
Branch: refs/heads/master
Commit: d7fe398693ac5bfc9e450c27e9afdc9d9de3dd62
Parents: 41b090f
Author: Kalyan Kumar Kalvagadda <kk...@cloudera.com>
Authored: Fri Dec 14 09:47:28 2018 -0600
Committer: Kalyan Kumar Kalvagadda <kk...@cloudera.com>
Committed: Fri Dec 14 10:31:37 2018 -0600
----------------------------------------------------------------------
.../core/common/utils/SentryConstants.java | 3 +
.../sentry/service/common/ServiceConstants.java | 6 +
.../db/service/model/MAuthzPathsMapping.java | 164 ++++++++++++++++---
.../sentry/provider/db/service/model/MPath.java | 8 +
.../provider/db/service/model/package.jdo | 28 +++-
.../service/persistent/QueryParamBuilder.java | 19 +++
.../db/service/persistent/SentryStore.java | 109 +++++++-----
.../db/service/persistent/TestSentryStore.java | 58 ++++++-
8 files changed, 315 insertions(+), 80 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java
index 9c2ba6f..8b53877 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java
@@ -73,4 +73,7 @@ public class SentryConstants {
// Representation for empty HMS snapshots not found on MAuthzPathsSnapshotId
public static final long EMPTY_PATHS_SNAPSHOT_ID = 0L;
+ // Representation for empty HMS Objects on MAuthzPathsMapping
+ public static final long EMPTY_PATHS_MAPPING_ID = 0L;
+
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
index 26a58f6..4508361 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
@@ -264,6 +264,12 @@ public class ServiceConstants {
*/
public static final String SENTRY_DB_VALUE_GENERATION_ALLOCATION_SIZE = "sentry.db.valuegeneration.allocation.size";
public static final int SENTRY_DB_VALUE_GENERATION_ALLOCATION_SIZE_DEFAULT = 100;
+
+ /**
+ * This value sets the maximum number of statements that can be included in a batch by datanucleus
+ */
+ public static final String SENTRY_STATEMENT_BATCH_LIMIT = "sentry.statement.batch.limit";
+ public static final int SENTRY_STATEMENT_BATCH_LIMIT_DEFAULT = 100;
}
public static class ClientConfig {
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
index c51f25a..95f4b4b 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
@@ -18,34 +18,57 @@
package org.apache.sentry.provider.db.service.model;
+import javax.jdo.PersistenceManager;
+import javax.jdo.Query;
+import javax.jdo.annotations.NotPersistent;
import javax.jdo.annotations.PersistenceCapable;
+import java.util.Set;
+import java.util.HashSet;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.Collections;
+
+import org.apache.sentry.provider.db.service.persistent.QueryParamBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* Transactional database backend for storing HMS Paths Updates. Any changes to this object
* require re-running the maven build so DN can re-enhance.
+ *
*/
@PersistenceCapable
public class MAuthzPathsMapping {
+ private static final Logger LOGGER = LoggerFactory
+ .getLogger(MAuthzPathsMapping.class);
+
+ private long authzObjectId;
private long authzSnapshotID;
private String authzObjName;
- private Set<MPath> paths;
+ // Used to fetch the paths from the sentry backend store.
+ private Set<MPath> pathsPersisted;
+ @NotPersistent
+ // Used to add paths.
+ private Set<String> pathsToPersist;
private long createTimeMs;
- public MAuthzPathsMapping(long authzSnapshotID, String authzObjName, Collection<String> paths) {
+ public MAuthzPathsMapping(long authzSnapshotID, long authzObjectId, String authzObjName, Collection<String> paths) {
this.authzSnapshotID = authzSnapshotID;
+ this.authzObjectId = authzObjectId;
this.authzObjName = MSentryUtil.safeIntern(authzObjName);
- this.paths = new HashSet<>(paths.size());
- for (String path : paths) {
- this.paths.add(new MPath(path));
+ this.pathsPersisted = Collections.EMPTY_SET;
+ this.pathsToPersist = new HashSet<>(paths.size());
+ for(String path : paths) {
+ this.pathsToPersist.add(path);
}
this.createTimeMs = System.currentTimeMillis();
}
+ public long getAuthzObjectId() {
+ return authzObjectId;
+ }
+
public long getCreateTime() {
return createTimeMs;
}
@@ -62,20 +85,18 @@ public class MAuthzPathsMapping {
this.authzObjName = authzObjName;
}
- public void setPaths(Set<MPath> paths) {
- this.paths = paths;
- }
-
- public Set<MPath> getPaths() {
- return paths;
- }
-
- public boolean removePath(MPath path) {
- return paths.remove(path);
+ public Set<MPath> getPathsPersisted() {
+ return pathsPersisted;
}
- public void addPath(MPath path) {
- paths.add(path);
+ public void addPathToPersist(Collection<String> paths) {
+ if(paths == null || paths.size() == 0) {
+ return;
+ }
+ if(pathsToPersist == null) {
+ pathsToPersist = new HashSet<>(paths.size());
+ }
+ pathsToPersist.addAll(paths);
}
/**
@@ -85,8 +106,8 @@ public class MAuthzPathsMapping {
* @param path the given path name
* @return an Path object that has the given path value.
*/
- public MPath getPath(String path) {
- for (MPath mPath : paths) {
+ public MPath getPathsPersisted(String path) {
+ for (MPath mPath : pathsPersisted) {
if (mPath.getPath().equals(path)) {
return mPath;
}
@@ -98,8 +119,8 @@ public class MAuthzPathsMapping {
* @return collection of paths strings contained in this object.
*/
public Collection<String> getPathStrings() {
- Collection<String> pathValues = new ArrayList<>(this.paths.size());
- for (MPath path : this.paths) {
+ Collection<String> pathValues = new ArrayList<>(this.pathsPersisted.size());
+ for (MPath path : this.pathsPersisted) {
pathValues.add(path.getPath());
}
return pathValues;
@@ -107,8 +128,10 @@ public class MAuthzPathsMapping {
@Override
public String toString() {
- return "MSentryPathsUpdate authzSnapshotID=[" + authzSnapshotID + "], authzObj=[" + authzObjName
- + "], paths=[" + paths.toString() + "], createTimeMs=[" + createTimeMs + "]";
+ return "MSentryPathsUpdate authzSnapshotID=[" + authzSnapshotID + "], authzObjectId=[" + authzObjectId + "]," +
+ "authzObj=[" + authzObjName + "], mPaths=[" + ((pathsPersisted != null) ? pathsPersisted.toString() : "")
+ + "], paths=[" + ((pathsToPersist != null) ? pathsToPersist.toString() : "") + "]," +
+ "createTimeMs=[" + createTimeMs + "]";
}
@Override
@@ -140,8 +163,97 @@ public class MAuthzPathsMapping {
return false;
} else if (authzSnapshotID != other.authzSnapshotID) {
return false;
- }
+ } else if (authzObjectId != other.authzObjectId) {
+ return false;
+ }
return true;
}
+
+ /**
+ * Persist the MAuthzPathsMapping by inserting entries into AUTHZ_PATH table in batches.
+ * @param pm Persistence manager instance
+ */
+ public void makePersistent(PersistenceManager pm) {
+ pm.makePersistent(this);
+ for (String path : pathsToPersist) {
+ pm.makePersistent(new MPathToPersist(authzObjectId, path));
+ }
+ pathsToPersist.clear();
+ }
+
+ /**
+ * Delete the paths associated with MAuthzPathsMapping entry.
+ * @param pm Persistence manager instance
+ * @param paths paths to be removed.
+ */
+ public void deletePersistent(PersistenceManager pm, Iterable<String> paths) {
+ Query query = pm.newQuery(MPathToPersist.class);
+ QueryParamBuilder paramBuilder = QueryParamBuilder.addPathFilter(null, paths).addObject("authzObjectId",
+ authzObjectId);
+ query.setFilter(paramBuilder.toString());
+ long delCount = query.deletePersistentAll(paramBuilder.getArguments());
+ LOGGER.debug("Entries deleted are %d", delCount);
+ }
+
+ /**
+ * There are performance issues in persisting instances of MPath at bulk because of the FK mapping with
+ * MAuthzPathsMapping. This FK mapping limits datanucleus from inserting these entries in batches.
+ * MPathToPersist is introduced to solve this performance issue.
+ * Note: Only {@link MPathToPersist} should be used to persist paths instead of {@link MPath} otherwise there
+ * will be duplicate entry exceptions as both JDO's MPath and {@link MPathToPersist}
+ * maintain their own sequence for PATH_ID column.
+ */
+ @PersistenceCapable
+ public static class MPathToPersist {
+ private String path;
+ private long authzObjectId;
+
+
+ public MPathToPersist(long authzObjectId, String path) {
+ this.authzObjectId = authzObjectId;
+ this.path = MSentryUtil.safeIntern(path);
+ }
+
+ public String getPath() {
+ return path;
+ }
+
+ public void setPath(String path) {
+ this.path = path;
+ }
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((path == null) ? 0 : path.hashCode());
+ return result;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+
+ if (obj == null) {
+ return false;
+ }
+
+ if (getClass() != obj.getClass()) {
+ return false;
+ }
+
+ MPathToPersist other = (MPathToPersist) obj;
+
+ if (path == null) {
+ return other.path == null;
+ } else if (authzObjectId != other.authzObjectId) {
+ return false;
+ }
+
+ return path.equals(other.path);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
index b0eaff2..ef2bf6c 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java
@@ -22,6 +22,14 @@ package org.apache.sentry.provider.db.service.model;
*
* New class is created for path in order to have 1 to many mapping
* between path and Authorizable object.
+ *
+ * There are performance issues in persisting instances of MPath at bulk because of the FK mapping with
+ * MAuthzPathsMapping. This FK mapping limits datanucleus from inserting these entries in batches.
+ * MPathToPersist was introduced to solve this performance issue.
+ * Note: This class should not be used to insert paths, instead {@link MAuthzPathsMapping.MPathToPersist} should be used.
+ * If MPath is used to persist paths, there will be duplicate entry exceptions as both JDO's MPath and
+ * {@link MAuthzPathsMapping.MPathToPersist} maintain their own sequence for PATH_ID column.
+
*/
public class MPath {
private String path;
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
index e3ae24b..4b27777 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
@@ -255,10 +255,10 @@
</field>
</class>
- <class name="MAuthzPathsMapping" identity-type="datastore" table="AUTHZ_PATHS_MAPPING" detachable="true">
- <datastore-identity strategy="increment">
- <column name="AUTHZ_OBJ_ID"/>
- </datastore-identity>
+ <class name="MAuthzPathsMapping" identity-type="application" table="AUTHZ_PATHS_MAPPING" detachable="true">
+ <field name="authzObjectId" primary-key="true">
+ <column name="AUTHZ_OBJ_ID" jdbc-type="BIGINT" allows-null="false"/>
+ </field>
<index name="AUTHZ_SNAPSHOT_ID_INDEX" unique="false">
<field name="authzSnapshotID"/>
</index>
@@ -273,7 +273,7 @@
<field name="createTimeMs">
<column name="CREATE_TIME_MS" jdbc-type="BIGINT"/>
</field>
- <field name = "paths">
+ <field name = "pathsPersisted">
<!-- Setting attribute dependent-element to true enables JDO cascading operations. in this case we need it to
cascade delete.
-->
@@ -283,7 +283,7 @@
</element>
</field>
<fetch-group name="includingPaths">
- <field name="paths"/>
+ <field name="pathsPersisted"/>
</fetch-group>
<field name="authzSnapshotID">
<column name="AUTHZ_SNAPSHOT_ID" jdbc-type="BIGINT" allows-null="false"/>
@@ -299,6 +299,22 @@
</field>
</class>
+ <!--
+ MPath has external foreign keys with MAuthzPathsMapping which limits it from persisting
+ in batches. MPathToPersist is replica of MPath with out the external foreign keys.
+ -->
+ <class name="MAuthzPathsMapping$MPathToPersist" identity-type="datastore" table="AUTHZ_PATH" detachable="true">
+ <datastore-identity strategy="increment">
+ <column name="PATH_ID"/>
+ </datastore-identity>
+ <field name="authzObjectId">
+ <column name="AUTHZ_OBJ_ID" jdbc-type="BIGINT" allows-null="false"/>
+ </field>
+ <field name="path">
+ <column name="PATH_NAME" length="4000" jdbc-type="VARCHAR"/>
+ </field>
+
</class>
+
<class name="MSentryPermChange" table="SENTRY_PERM_CHANGE" identity-type="application" detachable="true">
<field name="changeID" primary-key="true">
<column name="CHANGE_ID" jdbc-type="BIGINT" allows-null="false"/>
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
index f5802d7..11c208d 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
@@ -345,6 +345,25 @@ public class QueryParamBuilder {
}
/**
+ * Add common filter for set of paths. This is used to simplify creating filters for
+ * a collections of paths
+ * @param paramBuilder paramBuilder for parameters
+ * @param paths set paths
+ * @return paramBuilder supplied or a new one if the supplied one is null.
+ */
+ public static QueryParamBuilder addPathFilter(QueryParamBuilder paramBuilder,
+ Iterable<String> paths) {
+ if (paramBuilder == null) {
+ paramBuilder = new QueryParamBuilder();
+ }
+ if (paths == null) {
+ return paramBuilder;
+ }
+ paramBuilder.newChild().addSet("this.path == ", paths, false);
+ return paramBuilder;
+ }
+
+ /**
* Add multiple conditions for set of values.
* <p>
* Example:
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
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 dcf4651..63f5375 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
@@ -25,6 +25,7 @@ import static org.apache.sentry.core.common.utils.SentryConstants.DB_NAME;
import static org.apache.sentry.core.common.utils.SentryConstants.EMPTY_CHANGE_ID;
import static org.apache.sentry.core.common.utils.SentryConstants.EMPTY_NOTIFICATION_ID;
import static org.apache.sentry.core.common.utils.SentryConstants.EMPTY_PATHS_SNAPSHOT_ID;
+import static org.apache.sentry.core.common.utils.SentryConstants.EMPTY_PATHS_MAPPING_ID;
import static org.apache.sentry.core.common.utils.SentryConstants.GRANT_OPTION;
import static org.apache.sentry.core.common.utils.SentryConstants.INDEX_GROUP_ROLES_MAP;
import static org.apache.sentry.core.common.utils.SentryConstants.INDEX_USER_ROLES_MAP;
@@ -113,6 +114,7 @@ import static org.apache.sentry.core.common.utils.SentryConstants.TABLE_NAME;
import static org.apache.sentry.core.common.utils.SentryConstants.URI;
import static org.apache.sentry.core.common.utils.SentryUtils.isNULL;
import static org.apache.sentry.hdfs.Updateable.Update;
+import static org.apache.sentry.service.common.ServiceConstants.ServerConfig.SENTRY_STATEMENT_BATCH_LIMIT;
/**
* SentryStore is the data access object for Sentry data. Strings
@@ -250,6 +252,10 @@ public class SentryStore implements SentryStoreInterface {
// Disallow operations outside of transactions
prop.setProperty("datanucleus.NontransactionalRead", "false");
prop.setProperty("datanucleus.NontransactionalWrite", "false");
+ int batchSize = conf.getInt(SENTRY_STATEMENT_BATCH_LIMIT, ServerConfig.
+ SENTRY_STATEMENT_BATCH_LIMIT_DEFAULT);
+ prop.setProperty("datanucleus.rdbms.statementBatchLimit", Integer.toString(batchSize));
+
int allocationSize = conf.getInt(ServerConfig.SENTRY_DB_VALUE_GENERATION_ALLOCATION_SIZE, ServerConfig.
SENTRY_DB_VALUE_GENERATION_ALLOCATION_SIZE_DEFAULT);
prop.setProperty("datanucleus.valuegeneration.increment.allocationSize", Integer.toString(allocationSize));
@@ -3309,12 +3315,12 @@ public class SentryStore implements SentryStoreInterface {
pm.setDetachAllOnCommit(false); // No need to detach objects
deleteNotificationsSince(pm, notificationID + 1);
-
- // persist the notidicationID
+ // persist the notification ID
pm.makePersistent(new MSentryHmsNotification(notificationID));
// persist the full snapshot
long snapshotID = getCurrentAuthzPathsSnapshotID(pm);
+ long nextObjectId = getNextAuthzObjectID(pm);
long nextSnapshotID = snapshotID + 1;
pm.makePersistent(new MAuthzPathsSnapshotId(nextSnapshotID));
LOGGER.info("Attempting to commit new HMS snapshot with ID = {}", nextSnapshotID);
@@ -3322,8 +3328,9 @@ public class SentryStore implements SentryStoreInterface {
long lastProgressTime = System.currentTimeMillis();
for (Map.Entry<String, Collection<String>> authzPath : authzPaths.entrySet()) {
- pm.makePersistent(new MAuthzPathsMapping(nextSnapshotID, authzPath.getKey(), authzPath.getValue()));
-
+ MAuthzPathsMapping mapping = new MAuthzPathsMapping(nextSnapshotID, nextObjectId++, authzPath.getKey(),
+ authzPath.getValue());
+ mapping.makePersistent(pm);
objectsPersistedCount++;
pathsPersistedCount = pathsPersistedCount + authzPath.getValue().size();
@@ -3355,6 +3362,17 @@ public class SentryStore implements SentryStoreInterface {
}
/**
+ * Get the Next object ID to be persisted
+ * Always executed in the transaction context.
+ *
+ * @param pm The PersistenceManager object.
+ * @return the Next object ID to be persisted. It returns 0 if no rows are found.
+ */
+ private static long getNextAuthzObjectID(PersistenceManager pm) {
+ return getMaxPersistedIDCore(pm, MAuthzPathsMapping.class, "authzObjectId", EMPTY_PATHS_MAPPING_ID) + 1;
+ }
+
+ /**
* Get the last authorization path snapshot ID persisted.
* Always executed in the transaction context.
*
@@ -3373,7 +3391,8 @@ public class SentryStore implements SentryStoreInterface {
*
* @return the last persisted snapshot ID. It returns 0 if no rows are found.
*/
- private long getCurrentAuthzPathsSnapshotID() throws Exception {
+ @VisibleForTesting
+ long getCurrentAuthzPathsSnapshotID() throws Exception {
return tm.executeTransaction(
SentryStore::getCurrentAuthzPathsSnapshotID
);
@@ -3416,13 +3435,11 @@ public class SentryStore implements SentryStoreInterface {
MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, currentSnapshotID, authzObj);
if (mAuthzPathsMapping == null) {
- mAuthzPathsMapping = new MAuthzPathsMapping(currentSnapshotID, authzObj, paths);
+ mAuthzPathsMapping = new MAuthzPathsMapping(currentSnapshotID, getNextAuthzObjectID(pm), authzObj, paths);
} else {
- for (String path : paths) {
- mAuthzPathsMapping.addPath(new MPath(path));
- }
+ mAuthzPathsMapping.addPathToPersist(paths);
}
- pm.makePersistent(mAuthzPathsMapping);
+ mAuthzPathsMapping.makePersistent(pm);
}
/**
@@ -3460,16 +3477,7 @@ public class SentryStore implements SentryStoreInterface {
MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, currentSnapshotID, authzObj);
if (mAuthzPathsMapping != null) {
- for (String path : paths) {
- MPath mPath = mAuthzPathsMapping.getPath(path);
- if (mPath == null) {
- LOGGER.error("nonexistent path: {}", path);
- } else {
- mAuthzPathsMapping.removePath(mPath);
- pm.deletePersistent(mPath);
- }
- }
- pm.makePersistent(mAuthzPathsMapping);
+ mAuthzPathsMapping.deletePersistent(pm, paths);
} else {
LOGGER.error("nonexistent authzObj: {} on current paths snapshot ID #{}",
authzObj, currentSnapshotID);
@@ -3557,16 +3565,10 @@ public class SentryStore implements SentryStoreInterface {
MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, currentSnapshotID, oldObj);
if (mAuthzPathsMapping != null) {
- MPath mOldPath = mAuthzPathsMapping.getPath(oldPath);
- if (mOldPath == null) {
- LOGGER.error("nonexistent path: {}", oldPath);
- } else {
- mAuthzPathsMapping.removePath(mOldPath);
- pm.deletePersistent(mOldPath);
- }
- mAuthzPathsMapping.addPath(new MPath(newPath));
+ mAuthzPathsMapping.deletePersistent(pm,Collections.singleton(oldPath));
mAuthzPathsMapping.setAuthzObjName(newObj);
- pm.makePersistent(mAuthzPathsMapping);
+ mAuthzPathsMapping.addPathToPersist(Collections.singleton(newPath));
+ mAuthzPathsMapping.makePersistent(pm);
} else {
LOGGER.error("nonexistent authzObj: {} on current paths snapshot ID #{}",
oldObj, currentSnapshotID);
@@ -3703,24 +3705,40 @@ public class SentryStore implements SentryStoreInterface {
MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, currentSnapshotID, authzObj);
if (mAuthzPathsMapping == null) {
- mAuthzPathsMapping = new MAuthzPathsMapping(currentSnapshotID, authzObj, Sets.newHashSet(newPath));
+ mAuthzPathsMapping = new MAuthzPathsMapping(currentSnapshotID, getNextAuthzObjectID(pm), authzObj,
+ Collections.singleton(newPath));
} else {
- MPath mOldPath = mAuthzPathsMapping.getPath(oldPath);
- if (mOldPath == null) {
- LOGGER.error("nonexistent path: {}", oldPath);
- } else {
- mAuthzPathsMapping.removePath(mOldPath);
- pm.deletePersistent(mOldPath);
- }
- MPath mNewPath = new MPath(newPath);
- mAuthzPathsMapping.addPath(mNewPath);
+ mAuthzPathsMapping.deletePersistent(pm, Collections.singleton(oldPath));
+ mAuthzPathsMapping.addPathToPersist(Collections.singleton(newPath));
}
- pm.makePersistent(mAuthzPathsMapping);
+ mAuthzPathsMapping.makePersistent(pm);
}
/**
- * Get the MAuthzPathsMapping object from authzObj
+ * Get the Collection of MPath associated with snapshot id and authzObj
+ * @param authzSnapshotID Snapshot ID
+ * @param authzObj Object name
+ * @return Path mapping for object provided.
+ * @throws Exception
*/
+ @VisibleForTesting
+ Set<MPath> getMAuthzPaths(long authzSnapshotID, String authzObj) throws Exception {
+ return tm.executeTransactionWithRetry( pm -> {
+ MAuthzPathsMapping mapping = null;
+ pm.setDetachAllOnCommit(true); // No need to detach objects
+ mapping = getMAuthzPathsMappingCore(pm, authzSnapshotID, authzObj);
+ if(mapping != null) {
+ Set<MPath> paths = mapping.getPathsPersisted();
+ return paths;
+ } else {
+ return Collections.emptySet();
+ }
+ });
+ }
+
+ /**
+ * Get the MAuthzPathsMapping object from authzObj
+ */
private MAuthzPathsMapping getMAuthzPathsMappingCore(PersistenceManager pm,
long authzSnapshotID, String authzObj) {
Query query = pm.newQuery(MAuthzPathsMapping.class);
@@ -3782,6 +3800,15 @@ public class SentryStore implements SentryStoreInterface {
}
/**
+ * Get the total number of entries in AUTHZ_PATH table.
+ * @return number of entries in AUTHZ_PATH table.
+ */
+ @VisibleForTesting
+ long getPathCount() {
+ return getCount(MPath.class);
+ }
+
+ /**
* Method detects orphaned privileges
*
* @return True, If there are orphan privileges
http://git-wip-us.apache.org/repos/asf/sentry/blob/d7fe3986/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
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 ca8c416..b327e9e 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
@@ -19,6 +19,7 @@
package org.apache.sentry.provider.db.service.persistent;
import java.io.File;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -39,6 +40,7 @@ import java.util.concurrent.atomic.AtomicLong;
import com.google.common.collect.Lists;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang.StringUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.security.alias.CredentialProvider;
import org.apache.hadoop.security.alias.CredentialProviderFactory;
@@ -2473,11 +2475,44 @@ public class TestSentryStore extends org.junit.Assert {
assertTrue(CollectionUtils.isEqualCollection(Lists.newArrayList("/user/hive/warehouse/db2.db/table2.1",
"/user/hive/warehouse/db2.db/table2.3"),
pathImage.get("db2.table2")));
- assertEquals(6, sentryStore.getMPaths().size());
+ assertEquals(6, sentryStore.getPathCount());
assertEquals(notificationID, savedNotificationID);
}
@Test
+ public void testAddAuthzPathsMapping() throws Exception {
+ Set<MPath> paths;
+ // Persist an empty image so that we can add paths to it.
+ sentryStore.persistFullPathsImage(new HashMap<String, Collection<String>>(), 0);
+
+ // Check the latest persisted ID matches to both the path updates
+ long latestID = sentryStore.getCurrentAuthzPathsSnapshotID();
+
+
+ // Persist the path
+ sentryStore.addAuthzPathsMapping("db1.tb1", Arrays.asList("/hive/db1/tb1"), null);
+ paths = sentryStore.getMAuthzPaths(latestID, "db1.tb1");
+ // Verify that mapping is been added
+ assertEquals(paths.size(), 1);
+ assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1")));
+
+ // Add new path to an existing mapping
+ sentryStore.addAuthzPathsMapping("db1.tb1", Arrays.asList("/hive/db1/tb1/par1"), null);
+ paths = sentryStore.getMAuthzPaths(latestID, "db1.tb1");
+ // Verify that mapping is been updated with the new path
+ assertEquals(paths.size(), 2);
+ assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1/par1")));
+
+ // Add multiples path to an existing mapping
+ sentryStore.addAuthzPathsMapping("db1.tb1", Arrays.asList("/hive/db1/tb1/par2","/hive/db1/tb1/par3"), null);
+ paths = sentryStore.getMAuthzPaths(latestID, "db1.tb1");
+ // Verify that mapping is been updated with the new path
+ assertEquals(paths.size(), 4);
+ assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1/par2")));
+ assertTrue(paths.stream().anyMatch(x -> x.getPath().equalsIgnoreCase("/hive/db1/tb1/par3")));
+ }
+
+ @Test
public void testAddPathsWithDuplicatedNotificationIdShouldBeAllowed() throws Exception {
long notificationID = 1;
@@ -4142,30 +4177,39 @@ public class TestSentryStore extends org.junit.Assert {
@Test
public void testPersistFullPathsImageWithHugeData() throws Exception {
Map<String, Collection<String>> authzPaths = new HashMap<>();
- String[] prefixes = {"/user/hive/warehouse"};
+ String[] prefixes = {"/user/hive/warehouse/"};
+ int dbCount = 10, tableCount = 10, partitionCount = 10, prefixSize;
+ prefixSize = StringUtils.countMatches(prefixes[0], "/");
+
// Makes sure that authorizable object could be associated
// with different paths and can be properly persisted into database.
- for(int db_index = 1 ; db_index <= 10; db_index++) {
+ for(int db_index = 1 ; db_index <= dbCount; db_index++) {
String db_name = "db" + db_index;
- for( int table_index = 1; table_index <= 15; table_index++) {
+ for( int table_index = 1; table_index <= tableCount; table_index++) {
Set<String> paths = Sets.newHashSet();
String table_name = "tb" + table_index;
- String location = "/u/h/w/" + db_name + "/" + table_name;
- for (int part_index = 1; part_index <= 30; part_index++) {
+ String location = prefixes[0] + db_name + "/" + table_name;
+ paths.add(location);
+ for (int part_index = 1; part_index <= partitionCount; part_index++) {
paths.add(location + "/" + part_index);
}
authzPaths.put(db_name+table_name, paths);
}
}
long notificationID = 110000;
+ LOGGER.debug("Before " + Instant.now());
sentryStore.persistFullPathsImage(authzPaths, notificationID);
+ LOGGER.debug("After " + Instant.now());
PathsUpdate pathsUpdate = sentryStore.retrieveFullPathsImageUpdate(prefixes);
+ assertTrue(pathsUpdate.hasFullImage());
long savedNotificationID = sentryStore.getLastProcessedNotificationID();
assertEquals(1, pathsUpdate.getImgNum());
TPathsDump pathDump = pathsUpdate.toThrift().getPathsDump();
assertNotNull(pathDump);
Map<Integer, TPathEntry> nodeMap = pathDump.getNodeMap();
- assertTrue(nodeMap.size() > 0);
+ int nodeCount = prefixSize + dbCount + (dbCount * tableCount) +
+ (dbCount * tableCount * partitionCount);
+ assertTrue(nodeMap.size() == nodeCount);
assertEquals(notificationID, savedNotificationID);
}