You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by va...@apache.org on 2017/01/10 03:43:13 UTC

sentry git commit: SENTRY-1515: Cleanup exception handling in SentryStore (Alexander Kolbasov via Vamsee Yarlagadda, Reviewed by: Mat Crocker, Hao Hao, Vamsee Yarlagadda, Vadim Spector, Kalyan Kalvagadda)

Repository: sentry
Updated Branches:
  refs/heads/master 1a238c0ec -> 854934cb2


SENTRY-1515: Cleanup exception handling in SentryStore (Alexander Kolbasov via Vamsee Yarlagadda, Reviewed by: Mat Crocker, Hao Hao, Vamsee Yarlagadda, Vadim Spector, Kalyan Kalvagadda)


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

Branch: refs/heads/master
Commit: 854934cb218cf113bd089f51ca4fdfe72ecef0ee
Parents: 1a238c0
Author: Vamsee Yarlagadda <va...@cloudera.com>
Authored: Mon Jan 9 19:39:36 2017 -0800
Committer: Vamsee Yarlagadda <va...@cloudera.com>
Committed: Mon Jan 9 19:42:19 2017 -0800

----------------------------------------------------------------------
 .../java/org/apache/sentry/hdfs/Updateable.java |   2 +-
 .../org/apache/sentry/hdfs/SentryPlugin.java    |  61 +--
 .../org/apache/sentry/hdfs/UpdateForwarder.java |  31 +-
 .../sentry/hdfs/UpdateablePermissions.java      |   2 +-
 .../service/persistent/DelegateSentryStore.java |   2 +-
 .../service/persistent/SentryStoreLayer.java    |   2 +-
 .../db/service/persistent/SentryStore.java      | 517 ++++++++-----------
 7 files changed, 272 insertions(+), 345 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
index 4dc3a0c..8391b85 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
@@ -66,7 +66,7 @@ public interface Updateable<K extends Updateable.Update> {
    * @param currSeqNum
    * @return
    */
-  K createFullImageUpdate(long currSeqNum);
+  K createFullImageUpdate(long currSeqNum) throws Exception;
 
   String getUpdateableTypeName();
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
index 3695709..47c9f9d 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
@@ -127,36 +127,37 @@ public class SentryPlugin implements SentryPolicyStorePlugin, SigUtils.SigListen
     }
 
     @Override
-    public PermissionsUpdate retrieveFullImage(long currSeqNum) {
-      final Timer.Context timerContext =
-          SentryHdfsMetricsUtil.getRetrieveFullImageTimer.time();
-      Map<String, HashMap<String, String>> privilegeImage = sentryStore.retrieveFullPrivilegeImage();
-      Map<String, LinkedList<String>> roleImage = sentryStore.retrieveFullRoleImage();
-
-      TPermissionsUpdate tPermUpdate = new TPermissionsUpdate(true, currSeqNum,
-          new HashMap<String, TPrivilegeChanges>(),
-          new HashMap<String, TRoleChanges>());
-      for (Map.Entry<String, HashMap<String, String>> privEnt : privilegeImage.entrySet()) {
-        String authzObj = privEnt.getKey();
-        HashMap<String,String> privs = privEnt.getValue();
-        tPermUpdate.putToPrivilegeChanges(authzObj, new TPrivilegeChanges(
-            authzObj, privs, new HashMap<String, String>()));
-      }
-      for (Map.Entry<String, LinkedList<String>> privEnt : roleImage.entrySet()) {
-        String role = privEnt.getKey();
-        LinkedList<String> groups = privEnt.getValue();
-        tPermUpdate.putToRoleChanges(role, new TRoleChanges(role, groups, new LinkedList<String>()));
+    public PermissionsUpdate retrieveFullImage(long currSeqNum) throws Exception {
+      try(Timer.Context timerContext =
+                  SentryHdfsMetricsUtil.getRetrieveFullImageTimer.time()) {
+
+        SentryHdfsMetricsUtil.getRetrieveFullImageTimer.time();
+        Map<String, HashMap<String, String>> privilegeImage = sentryStore.retrieveFullPrivilegeImage();
+        Map<String, LinkedList<String>> roleImage = sentryStore.retrieveFullRoleImage();
+
+        TPermissionsUpdate tPermUpdate = new TPermissionsUpdate(true, currSeqNum,
+                new HashMap<String, TPrivilegeChanges>(),
+                new HashMap<String, TRoleChanges>());
+        for (Map.Entry<String, HashMap<String, String>> privEnt : privilegeImage.entrySet()) {
+          String authzObj = privEnt.getKey();
+          HashMap<String,String> privs = privEnt.getValue();
+          tPermUpdate.putToPrivilegeChanges(authzObj, new TPrivilegeChanges(
+                  authzObj, privs, new HashMap<String, String>()));
+        }
+        for (Map.Entry<String, LinkedList<String>> privEnt : roleImage.entrySet()) {
+          String role = privEnt.getKey();
+          LinkedList<String> groups = privEnt.getValue();
+          tPermUpdate.putToRoleChanges(role, new TRoleChanges(role, groups, new LinkedList<String>()));
+        }
+        PermissionsUpdate permissionsUpdate = new PermissionsUpdate(tPermUpdate);
+        permissionsUpdate.setSeqNum(currSeqNum);
+        SentryHdfsMetricsUtil.getPrivilegeChangesHistogram.update(
+                tPermUpdate.getPrivilegeChangesSize());
+        SentryHdfsMetricsUtil.getRoleChangesHistogram.update(
+                tPermUpdate.getRoleChangesSize());
+        return permissionsUpdate;
       }
-      PermissionsUpdate permissionsUpdate = new PermissionsUpdate(tPermUpdate);
-      permissionsUpdate.setSeqNum(currSeqNum);
-      timerContext.stop();
-      SentryHdfsMetricsUtil.getPrivilegeChangesHistogram.update(
-          tPermUpdate.getPrivilegeChangesSize());
-      SentryHdfsMetricsUtil.getRoleChangesHistogram.update(
-          tPermUpdate.getRoleChangesSize());
-      return permissionsUpdate;
     }
-
   }
 
   private UpdateForwarder<PathsUpdate> pathsUpdater;
@@ -221,7 +222,7 @@ public class SentryPlugin implements SentryPolicyStorePlugin, SigUtils.SigListen
    * Request for update from NameNode.
    * Full update to NameNode should happen only after full update from HMS.
    */
-  public List<PathsUpdate> getAllPathsUpdatesFrom(long pathSeqNum) {
+  public List<PathsUpdate> getAllPathsUpdatesFrom(long pathSeqNum) throws Exception {
     if (!fullUpdateNN.get()) {
       // Most common case - Sentry is NOT handling a full update.
       return pathsUpdater.getAllUpdatesFrom(pathSeqNum);
@@ -266,7 +267,7 @@ public class SentryPlugin implements SentryPolicyStorePlugin, SigUtils.SigListen
     }
   }
 
-  public List<PermissionsUpdate> getAllPermsUpdatesFrom(long permSeqNum) {
+  public List<PermissionsUpdate> getAllPermsUpdatesFrom(long permSeqNum) throws Exception {
     return permsUpdater.getAllUpdatesFrom(permSeqNum);
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
index 7387281..65b32e8 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
@@ -40,7 +40,7 @@ public class UpdateForwarder<K extends Updateable.Update> implements
 
   interface ExternalImageRetriever<K> {
 
-    K retrieveFullImage(long currSeqNum);
+    K retrieveFullImage(long currSeqNum) throws Exception;
 
   }
 
@@ -76,7 +76,8 @@ public class UpdateForwarder<K extends Updateable.Update> implements
       ExternalImageRetriever<K> imageRetreiver, int maxUpdateLogSize) {
     this(conf, updateable, imageRetreiver, maxUpdateLogSize, INIT_UPDATE_RETRY_DELAY);
   }
-  public UpdateForwarder(Configuration conf, Updateable<K> updateable, //NOPMD
+
+  protected UpdateForwarder(Configuration conf, Updateable<K> updateable, //NOPMD
       ExternalImageRetriever<K> imageRetreiver, int maxUpdateLogSize,
       int initUpdateRetryDelay) { 
     this.maxUpdateLogSize = maxUpdateLogSize;
@@ -143,7 +144,11 @@ public class UpdateForwarder<K extends Updateable.Update> implements
       initUpdater.start();
     }
     if (firstFullImage != null) {
-      appendToUpdateLog(firstFullImage);
+      try {
+        appendToUpdateLog(firstFullImage);
+      } catch (Exception e) {
+        LOGGER.warn("failed to update append log: ", e);
+      }
       this.updateable = updateable.updateFull(firstFullImage);
     }
   }
@@ -180,19 +185,27 @@ public class UpdateForwarder<K extends Updateable.Update> implements
           } else {
             // Retrieve full update from External Source and
             if (imageRetreiver != null) {
-              toUpdate = imageRetreiver
-                  .retrieveFullImage(update.getSeqNum());
+              try {
+                toUpdate = imageRetreiver
+                    .retrieveFullImage(update.getSeqNum());
+              } catch (Exception e) {
+                LOGGER.warn("failed to retrieve full image: ", e);
+              }
               updateable = updateable.updateFull(toUpdate);
             }
           }
         }
-        appendToUpdateLog(toUpdate);
+        try {
+          appendToUpdateLog(toUpdate);
+        } catch (Exception e) {
+          LOGGER.warn("failed to append to update log", e);
+        }
       }
     };
     updateHandler.execute(task);
   }
 
-  protected void appendToUpdateLog(K update) {
+  protected void appendToUpdateLog(K update) throws Exception {
     synchronized (getUpdateLog()) {
       boolean logCompacted = false;
       if (getMaxUpdateLogSize() > 0) {
@@ -222,7 +235,7 @@ public class UpdateForwarder<K extends Updateable.Update> implements
    * @param seqNum
    * @return
    */
-  public List<K> getAllUpdatesFrom(long seqNum) {
+  public List<K> getAllUpdatesFrom(long seqNum) throws Exception {
     List<K> retVal = new LinkedList<K>();
     synchronized (getUpdateLog()) {
       long currSeqNum = lastCommittedSeqNum.get();
@@ -310,7 +323,7 @@ public class UpdateForwarder<K extends Updateable.Update> implements
   }
 
   @Override
-  public K createFullImageUpdate(long currSeqNum) {
+  public K createFullImageUpdate(long currSeqNum) throws Exception {
     return (updateable != null) ? updateable.createFullImageUpdate(currSeqNum) : null;
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java
index 3d756c9..fe2baa6 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java
@@ -34,7 +34,7 @@ public class UpdateablePermissions implements Updateable<PermissionsUpdate>{
   }
 
   @Override
-  public PermissionsUpdate createFullImageUpdate(long currSeqNum) {
+  public PermissionsUpdate createFullImageUpdate(long currSeqNum) throws Exception {
     return imageRetreiver.retrieveFullImage(currSeqNum);
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
index 2ee06f9..57b6eff 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
@@ -97,7 +97,7 @@ public class DelegateSentryStore implements SentryStoreLayer {
   }
 
   @Override
-  public Set<String> getAllRoleNames() {
+  public Set<String> getAllRoleNames() throws Exception {
     return delegate.getAllRoleNames();
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
index f717f38..eec2757 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
@@ -162,7 +162,7 @@ public interface SentryStoreLayer {
    *
    * @returns The set of roles name,
    */
-  Set<String> getAllRoleNames();
+  Set<String> getAllRoleNames() throws Exception;
 
   /**
    * Get sentry privileges based on valid active roles and the authorize objects.

http://git-wip-us.apache.org/repos/asf/sentry/blob/854934cb/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 0a4b3e1..b7ae634 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
@@ -960,145 +960,128 @@ public class SentryStore {
         });
   }
 
-  private boolean hasAnyServerPrivileges(final Set<String> roleNames, final String serverName) {
+  private boolean hasAnyServerPrivileges(final Set<String> roleNames, final String serverName) throws Exception {
     if (roleNames == null || roleNames.isEmpty()) {
       return false;
     }
-    boolean result = false;
-    try {
-      result = (Boolean) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              Query query = pm.newQuery(MSentryPrivilege.class);
-              QueryParamBuilder paramBuilder = addRolesFilter(query,null, roleNames);
-              paramBuilder.add(SERVER_NAME, serverName);
-              query.setFilter(paramBuilder.toString());
-              query.setResult("count(this)");
-              Long numPrivs = (Long) query.executeWithMap(paramBuilder.getArguments());
-              return numPrivs > 0;
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+    return (Boolean) tm.executeTransaction(
+      new TransactionBlock() {
+        public Object execute(PersistenceManager pm) throws Exception {
+          Query query = pm.newQuery(MSentryPrivilege.class);
+          QueryParamBuilder paramBuilder = addRolesFilter(query,null, roleNames);
+          paramBuilder.add(SERVER_NAME, serverName);
+          query.setFilter(paramBuilder.toString());
+          query.setResult("count(this)");
+          Long numPrivs = (Long) query.executeWithMap(paramBuilder.getArguments());
+          return numPrivs > 0;
+        }
+      });
   }
 
   @SuppressWarnings("unchecked")
   private List<MSentryPrivilege> getMSentryPrivileges(final Set<String> roleNames,
-                                                      final TSentryAuthorizable authHierarchy) {
-    List<MSentryPrivilege> result = new ArrayList<>();
+                                                      final TSentryAuthorizable authHierarchy) throws Exception {
+    List<MSentryPrivilege> result = new ArrayList<MSentryPrivilege>();
     if (roleNames == null || roleNames.isEmpty()) {
       return result;
     }
 
-    try {
-      result = (List<MSentryPrivilege>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              Query query = pm.newQuery(MSentryPrivilege.class);
-              QueryParamBuilder paramBuilder = addRolesFilter(query, null, roleNames);
-
-              if (authHierarchy != null && authHierarchy.getServer() != null) {
-                paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
-                if (authHierarchy.getDb() != null) {
-                  paramBuilder.addNull(URI)
-                          .newChild()
-                            .add(DB_NAME, authHierarchy.getDb())
-                            .addNull(DB_NAME);
-                  if (authHierarchy.getTable() != null
-                      && !AccessConstants.ALL.equalsIgnoreCase(authHierarchy.getTable())) {
-                    if (!AccessConstants.SOME.equalsIgnoreCase(authHierarchy.getTable())) {
-                      paramBuilder.addNull(URI)
-                              .newChild()
-                                .add(TABLE_NAME, authHierarchy.getTable())
-                                .addNull(TABLE_NAME);
-                    }
-                    if (authHierarchy.getColumn() != null
-                        && !AccessConstants.ALL.equalsIgnoreCase(authHierarchy.getColumn())
-                        && !AccessConstants.SOME.equalsIgnoreCase(authHierarchy.getColumn())) {
-                      paramBuilder.addNull(URI)
-                              .newChild()
-                                .add(COLUMN_NAME, authHierarchy.getColumn())
-                                .addNull(COLUMN_NAME);
-                    }
+      return (List<MSentryPrivilege>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            Query query = pm.newQuery(MSentryPrivilege.class);
+            QueryParamBuilder paramBuilder = addRolesFilter(query, null, roleNames);
+
+            if (authHierarchy != null && authHierarchy.getServer() != null) {
+              paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
+              if (authHierarchy.getDb() != null) {
+                paramBuilder.addNull(URI)
+                        .newChild()
+                          .add(DB_NAME, authHierarchy.getDb())
+                          .addNull(DB_NAME);
+                if (authHierarchy.getTable() != null
+                    && !AccessConstants.ALL.equalsIgnoreCase(authHierarchy.getTable())) {
+                  if (!AccessConstants.SOME.equalsIgnoreCase(authHierarchy.getTable())) {
+                    paramBuilder.addNull(URI)
+                            .newChild()
+                              .add(TABLE_NAME, authHierarchy.getTable())
+                              .addNull(TABLE_NAME);
                   }
-                }
-                if (authHierarchy.getUri() != null) {
-                  paramBuilder.addNull(DB_NAME)
-                          .newChild()
-                            .addNull(URI)
+                  if (authHierarchy.getColumn() != null
+                      && !AccessConstants.ALL.equalsIgnoreCase(authHierarchy.getColumn())
+                      && !AccessConstants.SOME.equalsIgnoreCase(authHierarchy.getColumn())) {
+                    paramBuilder.addNull(URI)
                             .newChild()
-                              .addNotNull(URI)
-                              .addCustomParam("\"" + authHierarchy.getUri() +
-                                      "\".startsWith(:URI)", URI, authHierarchy.getUri());
+                              .add(COLUMN_NAME, authHierarchy.getColumn())
+                              .addNull(COLUMN_NAME);
+                  }
                 }
               }
-              LOGGER.debug("getMSentryPrivileges1() Query: " + paramBuilder.toString());
-              query.setFilter(paramBuilder.toString());
-              return query.executeWithMap(paramBuilder.getArguments());
+              if (authHierarchy.getUri() != null) {
+                paramBuilder.addNull(DB_NAME)
+                        .newChild()
+                          .addNull(URI)
+                          .newChild()
+                            .addNotNull(URI)
+                            .addCustomParam("\"" + authHierarchy.getUri() +
+                                    "\".startsWith(:URI)", URI, authHierarchy.getUri());
+              }
             }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+            LOGGER.debug("getMSentryPrivileges1() Query: " + paramBuilder.toString());
+            query.setFilter(paramBuilder.toString());
+            return query.executeWithMap(paramBuilder.getArguments());
+          }
+        });
   }
 
   @SuppressWarnings("unchecked")
   List<MSentryPrivilege> getMSentryPrivilegesByAuth(final Set<String> roleNames,
-      final TSentryAuthorizable authHierarchy) {
-    List<MSentryPrivilege> result = new ArrayList<MSentryPrivilege>();
-    try {
-      result = (List<MSentryPrivilege>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              Query query = pm.newQuery(MSentryPrivilege.class);
-              QueryParamBuilder paramBuilder = new QueryParamBuilder();
-              if (roleNames == null || roleNames.isEmpty()) {
-                paramBuilder.addString("!roles.isEmpty()");
-              } else {
-                addRolesFilter(query, paramBuilder, roleNames);
-              }
-              if (authHierarchy.getServer() != null) {
-                paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
-                if (authHierarchy.getDb() != null) {
-                  paramBuilder.add(DB_NAME, authHierarchy.getDb()).addNull(URI);
-                  if (authHierarchy.getTable() != null) {
-                    paramBuilder.add(TABLE_NAME, authHierarchy.getTable());
-                  } else {
-                    paramBuilder.addNull(TABLE_NAME);
-                  }
-                } else if (authHierarchy.getUri() != null) {
-                  paramBuilder.addNotNull(URI)
-                          .addNull(DB_NAME)
-                          .addCustomParam("(:authURI.startsWith(URI))", "authURI", authHierarchy.getUri());
+      final TSentryAuthorizable authHierarchy) throws Exception {
+      return (List<MSentryPrivilege>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            Query query = pm.newQuery(MSentryPrivilege.class);
+            QueryParamBuilder paramBuilder = new QueryParamBuilder();
+            if (roleNames == null || roleNames.isEmpty()) {
+              paramBuilder.addString("!roles.isEmpty()");
+            } else {
+              addRolesFilter(query, paramBuilder, roleNames);
+            }
+            if (authHierarchy.getServer() != null) {
+              paramBuilder.add(SERVER_NAME, authHierarchy.getServer());
+              if (authHierarchy.getDb() != null) {
+                paramBuilder.add(DB_NAME, authHierarchy.getDb()).addNull(URI);
+                if (authHierarchy.getTable() != null) {
+                  paramBuilder.add(TABLE_NAME, authHierarchy.getTable());
                 } else {
-                  paramBuilder.addNull(DB_NAME)
-                        .addNull(URI);
+                  paramBuilder.addNull(TABLE_NAME);
                 }
+              } else if (authHierarchy.getUri() != null) {
+                paramBuilder.addNotNull(URI)
+                        .addNull(DB_NAME)
+                        .addCustomParam("(:authURI.startsWith(URI))", "authURI", authHierarchy.getUri());
               } else {
-                // if no server, then return empty resultset
-                return new ArrayList<MSentryPrivilege>();
+                paramBuilder.addNull(DB_NAME)
+                      .addNull(URI);
               }
-              FetchGroup grp = pm.getFetchGroup(MSentryPrivilege.class, "fetchRole");
-              grp.addMember("roles");
-              pm.getFetchPlan().addGroup("fetchRole");
-              // LOGGER.debug("XXX: " + paramBuilder.toString());
-              query.setFilter(paramBuilder.toString());
-              return query.executeWithMap(paramBuilder.getArguments());
+            } else {
+              // if no server, then return empty resultset
+              return new ArrayList<MSentryPrivilege>();
             }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+            FetchGroup grp = pm.getFetchGroup(MSentryPrivilege.class, "fetchRole");
+            grp.addMember("roles");
+            pm.getFetchPlan().addGroup("fetchRole");
+            // LOGGER.debug("XXX: " + paramBuilder.toString());
+            query.setFilter(paramBuilder.toString());
+            return query.executeWithMap(paramBuilder.getArguments());
+          }
+        });
   }
 
   public TSentryPrivilegeMap listSentryPrivilegesByAuthorizable(Set<String> groups,
       TSentryActiveRoleSet activeRoles,
       TSentryAuthorizable authHierarchy, boolean isAdmin)
-      throws SentryInvalidInputException {
+          throws Exception {
     Map<String, Set<TSentryPrivilege>> resultPrivilegeMap = Maps.newTreeMap();
     Set<String> roles = getRolesToQuery(groups, null, new TSentryActiveRoleSet(true, null));
 
@@ -1159,7 +1142,7 @@ public class SentryStore {
 
   public Set<TSentryPrivilege> getTSentryPrivileges(Set<String> roleNames,
                                                     TSentryAuthorizable authHierarchy)
-          throws SentryInvalidInputException {
+          throws Exception {
     if (authHierarchy.getServer() == null) {
       throw new SentryInvalidInputException("serverName cannot be null !!");
     }
@@ -1225,23 +1208,17 @@ public class SentryStore {
   }
 
   @SuppressWarnings("unchecked")
-  public Set<String> getRoleNamesForGroups(final Set<String> groups) {
+  public Set<String> getRoleNamesForGroups(final Set<String> groups) throws Exception {
     if (groups == null || groups.isEmpty()) {
       return ImmutableSet.of();
     }
 
-    Set<String> result = new HashSet<>();
-    try {
-      result = (Set<String>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              return getRoleNamesForGroupsCore(pm, groups);
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+    return (Set<String>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            return getRoleNamesForGroupsCore(pm, groups);
+          }
+        });
   }
 
   private Set<String> getRoleNamesForGroupsCore(PersistenceManager pm, Set<String> groups) {
@@ -1249,23 +1226,17 @@ public class SentryStore {
   }
 
   @SuppressWarnings("unchecked")
-  public Set<String> getRoleNamesForUsers(final Set<String> users) {
+  public Set<String> getRoleNamesForUsers(final Set<String> users) throws Exception {
     if (users == null || users.isEmpty()) {
       return ImmutableSet.of();
     }
 
-    Set<String> result = new HashSet<>();
-    try {
-      result = (Set<String>) tm.executeTransaction(
+    return (Set<String>) tm.executeTransaction(
           new TransactionBlock() {
             public Object execute(PersistenceManager pm) throws Exception {
               return getRoleNamesForUsersCore(pm,users);
             }
           });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
   }
 
   private Set<String> getRoleNamesForUsersCore(PersistenceManager pm, Set<String> users) {
@@ -1273,23 +1244,16 @@ public class SentryStore {
   }
 
   @SuppressWarnings("unchecked")
-  public Set<TSentryRole> getTSentryRolesByUserNames(final Set<String> users) {
-    Set<TSentryRole> result = new HashSet<>();
-
-    try {
-      result = (Set<TSentryRole>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              Set<MSentryRole> mSentryRoles = getRolesForUsers(pm, users);
-              // Since {@link MSentryRole#getGroups()} is lazy-loading, the converting should be call
-              // before transaction committed.
-              return convertToTSentryRoles(mSentryRoles);
+  public Set<TSentryRole> getTSentryRolesByUserNames(final Set<String> users) throws Exception {
+    return (Set<TSentryRole>) tm.executeTransaction(
+        new TransactionBlock() {
+        public Object execute(PersistenceManager pm) throws Exception {
+            Set<MSentryRole> mSentryRoles = getRolesForUsers(pm, users);
+            // Since {@link MSentryRole#getGroups()} is lazy-loading, the converting should be call
+            // before transaction committed.
+            return convertToTSentryRoles(mSentryRoles);
             }
           });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
   }
 
   public Set<MSentryRole> getRolesForGroups(PersistenceManager pm, Set<String> groups) {
@@ -1323,13 +1287,13 @@ public class SentryStore {
   }
 
   Set<String> listAllSentryPrivilegesForProvider(Set<String> groups, Set<String> users,
-      TSentryActiveRoleSet roleSet) throws SentryInvalidInputException {
+      TSentryActiveRoleSet roleSet) throws Exception {
     return listSentryPrivilegesForProvider(groups, users, roleSet, null);
   }
 
 
   public Set<String> listSentryPrivilegesForProvider(Set<String> groups, Set<String> users,
-      TSentryActiveRoleSet roleSet, TSentryAuthorizable authHierarchy) throws SentryInvalidInputException {
+      TSentryActiveRoleSet roleSet, TSentryAuthorizable authHierarchy) throws Exception {
     Set<String> result = Sets.newHashSet();
     Set<String> rolesToQuery = getRolesToQuery(groups, users, roleSet);
     List<MSentryPrivilege> mSentryPrivileges = getMSentryPrivileges(rolesToQuery, authHierarchy);
@@ -1341,17 +1305,15 @@ public class SentryStore {
   }
 
   public boolean hasAnyServerPrivileges(Set<String> groups, Set<String> users,
-      TSentryActiveRoleSet roleSet, String server) {
+      TSentryActiveRoleSet roleSet, String server) throws Exception {
     Set<String> rolesToQuery = getRolesToQuery(groups, users, roleSet);
     return hasAnyServerPrivileges(rolesToQuery, server);
   }
 
   @SuppressWarnings("unchecked")
   private Set<String> getRolesToQuery(final Set<String> groups, final Set<String> users,
-      final TSentryActiveRoleSet roleSet) {
-    Set<String> result = new HashSet<>();
-    try {
-      result = (Set<String>) tm.executeTransaction(
+      final TSentryActiveRoleSet roleSet) throws Exception {
+      return (Set<String>) tm.executeTransaction(
           new TransactionBlock() {
             public Object execute(PersistenceManager pm) throws Exception {
               Set<String> activeRoleNames = toTrimedLower(roleSet.getRoles());
@@ -1363,10 +1325,6 @@ public class SentryStore {
                   roleNames);
             }
           });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
   }
 
   @VisibleForTesting
@@ -1859,83 +1817,73 @@ public class SentryStore {
   }
 
   @SuppressWarnings("unchecked")
-  public Map<String, HashMap<String, String>> retrieveFullPrivilegeImage() {
+  public Map<String, HashMap<String, String>> retrieveFullPrivilegeImage() throws Exception {
     Map<String, HashMap<String, String>> result = new HashMap<>();
-    try {
-      result = (Map<String, HashMap<String, String>>) tm.executeTransaction(
-              new TransactionBlock() {
-                public Object execute(PersistenceManager pm) throws Exception {
-                  Map<String, HashMap<String, String>> retVal = new HashMap<>();
-                  Query query = pm.newQuery(MSentryPrivilege.class);
-                  QueryParamBuilder paramBuilder = new QueryParamBuilder();
-                  paramBuilder
-                          .addNotNull(SERVER_NAME)
-                          .addNotNull(DB_NAME)
-                          .addNull(URI);
-
-                  query.setFilter(paramBuilder.toString());
-                  query.setOrdering("serverName ascending, dbName ascending, tableName ascending");
-                  List<MSentryPrivilege> privileges =
-                          (List<MSentryPrivilege>) query.executeWithMap(paramBuilder.getArguments());
-                  for (MSentryPrivilege mPriv : privileges) {
-                    String authzObj = mPriv.getDbName();
-                    if (!isNULL(mPriv.getTableName())) {
-                      authzObj = authzObj + "." + mPriv.getTableName();
-                    }
-                    HashMap<String, String> pUpdate = retVal.get(authzObj);
-                    if (pUpdate == null) {
-                      pUpdate = new HashMap<String, String>();
-                      retVal.put(authzObj, pUpdate);
-                    }
-                    for (MSentryRole mRole : mPriv.getRoles()) {
-                      String existingPriv = pUpdate.get(mRole.getRoleName());
-                      if (existingPriv == null) {
-                        pUpdate.put(mRole.getRoleName(), mPriv.getAction().toUpperCase());
-                      } else {
-                        pUpdate.put(mRole.getRoleName(), existingPriv + ","
-                                + mPriv.getAction().toUpperCase());
-                      }
-                    }
-                  }
-                  return retVal;
-                }
-              });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+    return (Map<String, HashMap<String, String>>) tm.executeTransaction(
+      new TransactionBlock() {
+        public Object execute(PersistenceManager pm) throws Exception {
+          Map<String, HashMap<String, String>> retVal = new HashMap<>();
+          Query query = pm.newQuery(MSentryPrivilege.class);
+          QueryParamBuilder paramBuilder = new QueryParamBuilder();
+          paramBuilder
+                  .addNotNull(SERVER_NAME)
+                  .addNotNull(DB_NAME)
+                  .addNull(URI);
+
+          query.setFilter(paramBuilder.toString());
+          query.setOrdering("serverName ascending, dbName ascending, tableName ascending");
+          List<MSentryPrivilege> privileges =
+                  (List<MSentryPrivilege>) query.executeWithMap(paramBuilder.getArguments());
+          for (MSentryPrivilege mPriv : privileges) {
+            String authzObj = mPriv.getDbName();
+            if (!isNULL(mPriv.getTableName())) {
+              authzObj = authzObj + "." + mPriv.getTableName();
+            }
+            HashMap<String, String> pUpdate = retVal.get(authzObj);
+            if (pUpdate == null) {
+              pUpdate = new HashMap<String, String>();
+              retVal.put(authzObj, pUpdate);
+            }
+            for (MSentryRole mRole : mPriv.getRoles()) {
+              String existingPriv = pUpdate.get(mRole.getRoleName());
+              if (existingPriv == null) {
+                pUpdate.put(mRole.getRoleName(), mPriv.getAction().toUpperCase());
+              } else {
+                pUpdate.put(mRole.getRoleName(), existingPriv + ","
+                        + mPriv.getAction().toUpperCase());
+              }
+            }
+          }
+          return retVal;
+        }
+      });
   }
 
   /**
    * @return Mapping of Role -> [Groups]
    */
   @SuppressWarnings("unchecked")
-  public Map<String, LinkedList<String>> retrieveFullRoleImage() {
+  public Map<String, LinkedList<String>> retrieveFullRoleImage() throws Exception {
     Map<String, LinkedList<String>> result = new HashMap<>();
-    try {
-      result = (Map<String, LinkedList<String>>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              Map<String, LinkedList<String>> retVal = new HashMap<>();
-              Query query = pm.newQuery(MSentryGroup.class);
-              List<MSentryGroup> groups = (List<MSentryGroup>) query.execute();
-              for (MSentryGroup mGroup : groups) {
-                for (MSentryRole role : mGroup.getRoles()) {
-                  LinkedList<String> rUpdate = retVal.get(role.getRoleName());
-                  if (rUpdate == null) {
-                    rUpdate = new LinkedList<String>();
-                    retVal.put(role.getRoleName(), rUpdate);
-                  }
-                  rUpdate.add(mGroup.getGroupName());
+    return (Map<String, LinkedList<String>>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            Map<String, LinkedList<String>> retVal = new HashMap<>();
+            Query query = pm.newQuery(MSentryGroup.class);
+            List<MSentryGroup> groups = (List<MSentryGroup>) query.execute();
+            for (MSentryGroup mGroup : groups) {
+              for (MSentryRole role : mGroup.getRoles()) {
+                LinkedList<String> rUpdate = retVal.get(role.getRoleName());
+                if (rUpdate == null) {
+                  rUpdate = new LinkedList<String>();
+                  retVal.put(role.getRoleName(), rUpdate);
                 }
+                rUpdate.add(mGroup.getGroupName());
               }
-              return retVal;
             }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+            return retVal;
+          }
+        });
   }
 
   /**
@@ -2132,10 +2080,9 @@ public class SentryStore {
 
   // get mapping datas for [group,role], [user,role] with the specific roles
   @SuppressWarnings("unchecked")
-  public List<Map<String, Set<String>>> getGroupUserRoleMapList(final Set<String> roleNames) {
+  public List<Map<String, Set<String>>> getGroupUserRoleMapList(final Set<String> roleNames) throws Exception {
     List<Map<String, Set<String>>> result = new ArrayList<>();
-    try {
-      result = (List<Map<String, Set<String>>>) tm.executeTransaction(
+      return (List<Map<String, Set<String>>>) tm.executeTransaction(
           new TransactionBlock() {
             public Object execute(PersistenceManager pm) throws Exception {
               Query query = pm.newQuery(MSentryRole.class);
@@ -2157,10 +2104,6 @@ public class SentryStore {
               return mapsList;
             }
           });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
   }
 
   private Map<String, Set<String>> getGroupRolesMap(List<MSentryRole> mSentryRoles) {
@@ -2262,19 +2205,13 @@ public class SentryStore {
    * @return Set of all role names, or an empty set if no roles are defined
    */
   @SuppressWarnings("unchecked")
-  public Set<String> getAllRoleNames() {
-    Set<String> result = new HashSet<>();
-    try {
-      return (Set<String>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              return getAllRoleNamesCore(pm);
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-      return  new HashSet<>();
-    }
+  public Set<String> getAllRoleNames() throws Exception {
+    return (Set<String>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            return getAllRoleNamesCore(pm);
+          }
+        });
   }
 
   /**
@@ -2350,79 +2287,55 @@ public class SentryStore {
 
   @VisibleForTesting
   @SuppressWarnings("unchecked")
-  protected Map<String, MSentryRole> getRolesMap() {
-    Map<String, MSentryRole> result = new HashMap<>();
-    try {
-      result = (Map<String, MSentryRole>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              List<MSentryRole> mSentryRoles = getAllRoles(pm);
-              Map<String, MSentryRole> existRolesMap = Maps.newHashMap();
-              if (mSentryRoles != null) {
-                // change the List<MSentryRole> -> Map<roleName, Set<MSentryRole>>
-                for (MSentryRole mSentryRole : mSentryRoles) {
-                  existRolesMap.put(mSentryRole.getRoleName(), mSentryRole);
-                }
+  protected Map<String, MSentryRole> getRolesMap() throws Exception {
+    return (Map<String, MSentryRole>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            List<MSentryRole> mSentryRoles = getAllRoles(pm);
+            Map<String, MSentryRole> existRolesMap = Maps.newHashMap();
+            if (mSentryRoles != null) {
+              // change the List<MSentryRole> -> Map<roleName, Set<MSentryRole>>
+              for (MSentryRole mSentryRole : mSentryRoles) {
+                existRolesMap.put(mSentryRole.getRoleName(), mSentryRole);
               }
-
-              return existRolesMap;
             }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+
+            return existRolesMap;
+          }
+        });
   }
 
   @VisibleForTesting
   @SuppressWarnings("unchecked")
-  protected Map<String, MSentryGroup> getGroupNameToGroupMap() {
-    Map<String, MSentryGroup>result = new HashMap<>();
-    try {
-      result = (Map<String, MSentryGroup>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              return getGroupNameTGroupMap(pm);
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+  protected Map<String, MSentryGroup> getGroupNameToGroupMap() throws Exception {
+    return (Map<String, MSentryGroup>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            return getGroupNameTGroupMap(pm);
+          }
+        });
   }
 
   @VisibleForTesting
   @SuppressWarnings("unchecked")
-  protected Map<String, MSentryUser> getUserNameToUserMap() {
-    Map<String, MSentryUser> result = new HashMap<>();
-    try {
-      result = (Map<String, MSentryUser>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              return getUserNameToUserMap(pm);
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+  protected Map<String, MSentryUser> getUserNameToUserMap() throws Exception {
+    return (Map<String, MSentryUser>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            return getUserNameToUserMap(pm);
+          }
+        });
   }
 
   @VisibleForTesting
   @SuppressWarnings("unchecked")
-  protected List<MSentryPrivilege> getPrivilegesList() {
-    List<MSentryPrivilege> result = new ArrayList<>();
-    try {
-      result = (List<MSentryPrivilege>) tm.executeTransaction(
-          new TransactionBlock() {
-            public Object execute(PersistenceManager pm) throws Exception {
-              return getPrivilegesList(pm);
-            }
-          });
-    } catch (Exception e) {
-      LOGGER.error(e.getMessage(), e);
-    }
-    return result;
+  protected List<MSentryPrivilege> getPrivilegesList() throws Exception {
+    return (List<MSentryPrivilege>) tm.executeTransaction(
+        new TransactionBlock() {
+          public Object execute(PersistenceManager pm) throws Exception {
+            return getPrivilegesList(pm);
+          }
+        });
   }
 
   /**