SENTRY-1768: Avoid detaching object on transaction exit when it isn't needed (Alex Kolbasov, reviewed by Na Li and Vamsee Yarlagadda)
Change-Id: Idec00b5caebba12524df5678b2e2b818b11fb1b6 Reviewed-on: http://gerrit.sjc.cloudera.com:8080/22791 Reviewed-by: Na Li <[email protected]> Tested-by: Jenkins User Reviewed-by: Alexander Kolbasov <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/49cc480b Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/49cc480b Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/49cc480b Branch: refs/for/cdh5-1.5.1_ha Commit: 49cc480b3c4326151e07cf7cd4d760ee7b1d49d1 Parents: e4f6f22 Author: Alexander Kolbasov <[email protected]> Authored: Wed May 17 13:19:30 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Wed May 17 15:16:19 2017 -0700 ---------------------------------------------------------------------- .../persistent/DeltaTransactionBlock.java | 1 + .../db/service/persistent/SentryStore.java | 47 +++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/49cc480b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java index e2c14e0..716ce09 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java @@ -70,6 +70,7 @@ public class DeltaTransactionBlock implements TransactionBlock<Object> { */ private void persistUpdate(PersistenceManager pm, Update update) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects Preconditions.checkNotNull(update); // persistUpdate cannot handle full image update, instead http://git-wip-us.apache.org/repos/asf/sentry/blob/49cc480b/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 fb648bb..0faaf28 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 @@ -190,8 +190,8 @@ public class SentryStore { if (prop.getProperty(ServerConfig.DATANUCLEUS_ISOLATION_LEVEL, ""). equals(ServerConfig.DATANUCLEUS_REPEATABLE_READ) && jdbcUrl.contains(oracleDb)) { - String parts[] = jdbcUrl.split(":"); - if (parts.length > 1 && parts[1].equals(oracleDb)) { + String[] parts = jdbcUrl.split(":"); + if ((parts.length > 1) && parts[1].equals(oracleDb)) { // For Oracle JDBC driver, replace "repeatable-read" with "read-committed" prop.setProperty(ServerConfig.DATANUCLEUS_ISOLATION_LEVEL, "read-committed"); @@ -636,11 +636,11 @@ public class SentryStore { MSentryPrivilege mSelect = getMSentryPrivilege(tNotAll, pm); tNotAll.setAction(AccessConstants.INSERT); MSentryPrivilege mInsert = getMSentryPrivilege(tNotAll, pm); - if ((mSelect != null) && (mRole.getPrivileges().contains(mSelect))) { + if ((mSelect != null) && mRole.getPrivileges().contains(mSelect)) { mSelect.removeRole(mRole); pm.makePersistent(mSelect); } - if ((mInsert != null) && (mRole.getPrivileges().contains(mInsert))) { + if ((mInsert != null) && mRole.getPrivileges().contains(mInsert)) { mInsert.removeRole(mRole); pm.makePersistent(mInsert); } @@ -2226,6 +2226,7 @@ public class SentryStore { new TransactionBlock<PermissionsImage>() { public PermissionsImage execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects // curChangeID could be 0, if Sentry server has been running before // enable SentryPlugin(HDFS Sync feature). long curChangeID = getLastProcessedChangeIDCore(pm, MSentryPermChange.class); @@ -2247,6 +2248,7 @@ public class SentryStore { */ private Map<String, Map<String, String>> retrieveFullPrivilegeImageCore(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects Map<String, Map<String, String>> retVal = new HashMap<>(); Query query = pm.newQuery(MSentryPrivilege.class); @@ -2292,6 +2294,7 @@ public class SentryStore { */ private Map<String, List<String>> retrieveFullRoleImageCore(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects Query query = pm.newQuery(MSentryGroup.class); @SuppressWarnings("unchecked") List<MSentryGroup> groups = (List<MSentryGroup>) query.execute(); @@ -2330,6 +2333,7 @@ public class SentryStore { public Object execute(PersistenceManager pm) throws Exception { // curChangeID could be 0 for the first full snapshot fetching // from HMS. It does not have corresponding delta update. + pm.setDetachAllOnCommit(false); // No need to detach objects long curChangeID = getLastProcessedChangeIDCore(pm, MSentryPathChange.class); Map<String, Set<String>> pathImage = retrieveFullPathsImageCore(pm); @@ -2366,6 +2370,7 @@ public class SentryStore { tm.executeTransactionWithRetry( new TransactionBlock() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects for (Map.Entry<String, Set<String>> authzPath : authzPaths.entrySet()) { createAuthzPathsMappingCore(pm, authzPath.getKey(), authzPath.getValue()); } @@ -2452,6 +2457,7 @@ public class SentryStore { final Update update) throws Exception { execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects deleteAuthzPathsMappingCore(pm, authzObj, paths); return null; } @@ -2473,7 +2479,7 @@ public class SentryStore { for (String path : paths) { MPath mPath = mAuthzPathsMapping.getPath(path); if (mPath == null) { - LOGGER.error("nonexistent path: " + path); + LOGGER.error("nonexistent path: {}", path); } else { mAuthzPathsMapping.removePath(mPath); pm.deletePersistent(mPath); @@ -2481,7 +2487,7 @@ public class SentryStore { } pm.makePersistent(mAuthzPathsMapping); } else { - LOGGER.error("nonexistent authzObj: " + authzObj); + LOGGER.error("nonexistent authzObj: {}", authzObj); } } @@ -2497,6 +2503,7 @@ public class SentryStore { throws Exception { execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects deleteAllAuthzPathsMappingCore(pm, authzObj); return null; } @@ -2519,7 +2526,7 @@ public class SentryStore { } pm.deletePersistent(mAuthzPathsMapping); } else { - LOGGER.error("nonexistent authzObj: " + authzObj); + LOGGER.error("nonexistent authzObj: {}", authzObj); } } @@ -2539,6 +2546,7 @@ public class SentryStore { final String oldPath, final String newPath, final Update update) throws Exception { execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects renameAuthzPathsMappingCore(pm, oldObj, newObj, oldPath, newPath); return null; } @@ -2563,7 +2571,7 @@ public class SentryStore { if (mAuthzPathsMapping != null) { MPath mOldPath = mAuthzPathsMapping.getPath(oldPath); if (mOldPath == null) { - LOGGER.error("nonexistent path: " + oldPath); + LOGGER.error("nonexistent path: {}", oldPath); } else { mAuthzPathsMapping.removePath(mOldPath); pm.deletePersistent(mOldPath); @@ -2572,7 +2580,7 @@ public class SentryStore { mAuthzPathsMapping.setAuthzObjName(newObj); pm.makePersistent(mAuthzPathsMapping); } else { - LOGGER.error("nonexistent authzObj: " + oldObj); + LOGGER.error("nonexistent authzObj: {}", oldObj); } } @@ -2589,6 +2597,7 @@ public class SentryStore { final Update update) throws Exception { execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects renameAuthzObjCore(pm, oldObj, newObj); return null; } @@ -2611,7 +2620,7 @@ public class SentryStore { mAuthzPathsMapping.setAuthzObjName(newObj); pm.makePersistent(mAuthzPathsMapping); } else { - LOGGER.error("nonexistent authzObj: " + oldObj); + LOGGER.error("nonexistent authzObj: {}", oldObj); } } @@ -2630,6 +2639,7 @@ public class SentryStore { final String newPath, final Update update) throws Exception { execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() { public Object execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects updateAuthzPathsMappingCore(pm, authzObj, oldPath, newPath); return null; } @@ -2656,7 +2666,7 @@ public class SentryStore { } else { MPath mOldPath = mAuthzPathsMapping.getPath(oldPath); if (mOldPath == null) { - LOGGER.error("nonexistent path: " + oldPath); + LOGGER.error("nonexistent path: {}", oldPath); } else { mAuthzPathsMapping.removePath(mOldPath); pm.deletePersistent(mOldPath); @@ -2711,9 +2721,9 @@ public class SentryStore { } Boolean findOrphanedPrivilegesCore(PersistenceManager pm) { - ArrayList<Object> idList = new ArrayList<Object>(); //Perform a SQL query to get things that look like orphans List<MSentryPrivilege> results = getAllMSentryPrivilegesCore(pm); + List<Object> idList = new ArrayList<>(results.size()); for (MSentryPrivilege orphan : results) { idList.add(pm.getObjectId(orphan)); } @@ -2826,6 +2836,7 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<Long>() { public Long execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects return getLastProcessedChangeIDCore(pm, MSentryPermChange.class); } }); @@ -2840,6 +2851,7 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<Long>() { public Long execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects return getLastProcessedChangeIDCore(pm, MSentryPathChange.class); } }); @@ -2855,8 +2867,8 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<Long>() { public Long execute(PersistenceManager pm) throws Exception { - long changeID = getLastProcessedChangeIDCore(pm, - MSentryPathChange.class); + pm.setDetachAllOnCommit(false); // No need to detach objects + long changeID = getLastProcessedChangeIDCore(pm, MSentryPathChange.class); if (changeID == EMPTY_CHANGE_ID) { return EMPTY_CHANGE_ID; } else { @@ -2972,6 +2984,7 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<Boolean>() { public Boolean execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects return changeExistsCore(pm, MSentryPermChange.class, changeID); } }); @@ -2988,6 +3001,7 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<Boolean>() { public Boolean execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects return changeExistsCore(pm, MSentryPathChange.class, changeID); } }); @@ -3041,8 +3055,7 @@ public class SentryStore { Query query = pm.newQuery(changeCls); query.setFilter("this.changeID >= t"); query.declareParameters("long t"); - List<T> changes = (List<T>) query.execute(changeID); - return changes; + return (List<T>) query.execute(changeID); } /** @@ -3059,6 +3072,7 @@ public class SentryStore { throws Exception { return tm.executeTransaction(new TransactionBlock<List<MSentryPathChange>>() { public List<MSentryPathChange> execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects List<MSentryPathChange> pathChanges = getMSentryChangesCore(pm, MSentryPathChange.class, changeID); long curChangeID = getLastProcessedChangeIDCore(pm, MSentryPathChange.class); @@ -3092,6 +3106,7 @@ public class SentryStore { return tm.executeTransaction( new TransactionBlock<List<MSentryPermChange>>() { public List<MSentryPermChange> execute(PersistenceManager pm) throws Exception { + pm.setDetachAllOnCommit(false); // No need to detach objects List<MSentryPermChange> permChanges = getMSentryChangesCore(pm, MSentryPermChange.class, changeID); long curChangeID = getLastProcessedChangeIDCore(pm, MSentryPermChange.class);
