sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ak...@apache.org
Subject sentry git commit: SENTRY-1806: Improve memory handling for HDFS sync (Alex Kolbasov, reviewed by Vamsee Yarlagadda, Misha Dmitriev, Kalyan Kalvagadda)
Date Tue, 20 Jun 2017 23:51:31 GMT
Repository: sentry
Updated Branches:
  refs/heads/sentry-ha-redesign 688c11668 -> a785d29c9


SENTRY-1806: Improve memory handling for HDFS sync (Alex Kolbasov, reviewed by Vamsee Yarlagadda,
Misha Dmitriev, Kalyan Kalvagadda)


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

Branch: refs/heads/sentry-ha-redesign
Commit: a785d29c9045842e8f9a502d7b102f5ece2ccf99
Parents: 688c116
Author: Alexander Kolbasov <akolb@cloudera.com>
Authored: Tue Jun 20 16:48:57 2017 -0700
Committer: Alexander Kolbasov <akolb@cloudera.com>
Committed: Tue Jun 20 16:48:57 2017 -0700

----------------------------------------------------------------------
 .../exception/SentryHdfsServiceException.java   |   7 +-
 .../org/apache/sentry/hdfs/DeltaRetriever.java  |   8 +-
 .../org/apache/sentry/hdfs/PathsUpdate.java     |  10 +-
 .../apache/sentry/hdfs/PermissionsUpdate.java   |  12 +--
 .../org/apache/sentry/hdfs/SentryUpdater.java   |   5 +-
 .../apache/sentry/hdfs/DBUpdateForwarder.java   |  36 +++----
 .../apache/sentry/hdfs/PathDeltaRetriever.java  |  17 +--
 .../apache/sentry/hdfs/PermDeltaRetriever.java  |   9 +-
 .../apache/sentry/hdfs/PermImageRetriever.java  |   8 +-
 .../sentry/hdfs/SentryHDFSServiceClient.java    |  18 +++-
 .../SentryHDFSServiceClientDefaultImpl.java     |  28 +++--
 .../sentry/hdfs/SentryHDFSServiceProcessor.java | 104 ++++++++++---------
 .../db/service/model/MAuthzPathsMapping.java    |   5 +-
 .../db/service/persistent/SentryStore.java      |   9 +-
 14 files changed, 148 insertions(+), 128 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
index 6b09dc2..efe4f76 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
@@ -18,16 +18,11 @@
 
 package org.apache.sentry.core.common.exception;
 
-public class SentryHdfsServiceException extends RuntimeException {
+public class SentryHdfsServiceException extends Exception {
   private static final long serialVersionUID = 1511645864949767378L;
 
   public SentryHdfsServiceException(String message, Throwable cause) {
     super(message, cause);
   }
 
-  public SentryHdfsServiceException(String message) {
-    super(message);
-  }
-
-
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java
b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java
index 0e58593..4503950 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/DeltaRetriever.java
@@ -18,7 +18,7 @@
 
 package org.apache.sentry.hdfs;
 
-import java.util.Collection;
+import java.util.List;
 
 import static org.apache.sentry.hdfs.Updateable.Update;
 
@@ -36,15 +36,15 @@ import static org.apache.sentry.hdfs.Updateable.Update;
 public interface DeltaRetriever<K extends Update> {
 
   /**
-   * Retrieves all delta updates of type {@link Update} newer than or equal with
+   * Retrieves all delta updates of type {@link Update} newer than or equal to
    * the given sequence number/change ID (inclusive) from a persistent storage.
    * An empty collection can be returned.
    *
    * @param seqNum the given seq number
-   * @return a collect of delta updates of type K
+   * @return delta updates of type K
    * @throws Exception when there is an error in operation on persistent storage
    */
-  Collection<K> retrieveDelta(long seqNum) throws Exception;
+  List<K> retrieveDelta(long seqNum) throws Exception;
 
   /**
    * Checks if there the delta update is available, given the sequence number/change

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
index 6b31f7a..49befee 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
@@ -20,7 +20,7 @@ package org.apache.sentry.hdfs;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -59,7 +59,7 @@ public class PathsUpdate implements Updateable.Update {
 
   public PathsUpdate(long seqNum, boolean hasFullImage) {
     tPathsUpdate = new TPathsUpdate(hasFullImage, seqNum,
-        new LinkedList<TPathChanges>());
+        new ArrayList<TPathChanges>());
   }
 
   @Override
@@ -70,12 +70,12 @@ public class PathsUpdate implements Updateable.Update {
   public TPathChanges newPathChange(String authzObject) {
 
     TPathChanges pathChanges = new TPathChanges(authzObject,
-        new LinkedList<List<String>>(), new LinkedList<List<String>>());
+        new ArrayList<List<String>>(), new ArrayList<List<String>>());
     tPathsUpdate.addToPathChanges(pathChanges);
     return pathChanges;
   }
 
-  public List<TPathChanges> getPathChanges() {
+  List<TPathChanges> getPathChanges() {
     return tPathsUpdate.getPathChanges();
   }
 
@@ -89,7 +89,7 @@ public class PathsUpdate implements Updateable.Update {
     tPathsUpdate.setSeqNum(seqNum);
   }
 
-  public TPathsUpdate toThrift() {
+  TPathsUpdate toThrift() {
     return tPathsUpdate;
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
index 14a4a0f..ebb0b96 100644
--- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
+++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
@@ -18,9 +18,9 @@
 package org.apache.sentry.hdfs;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.LinkedList;
 
 import org.apache.sentry.hdfs.service.thrift.TPermissionsUpdate;
 import org.apache.sentry.hdfs.service.thrift.TPrivilegeChanges;
@@ -80,21 +80,21 @@ public class PermissionsUpdate implements Updateable.Update {
     if (tPermUpdate.getRoleChanges().containsKey(role)) {
       return tPermUpdate.getRoleChanges().get(role);
     }
-    TRoleChanges roleUpdate = new TRoleChanges(role, new LinkedList<String>(),
-        new LinkedList<String>());
+    TRoleChanges roleUpdate = new TRoleChanges(role, new ArrayList<String>(),
+        new ArrayList<String>());
     tPermUpdate.getRoleChanges().put(role, roleUpdate);
     return roleUpdate;
   }
 
-  public Collection<TRoleChanges> getRoleUpdates() {
+  Collection<TRoleChanges> getRoleUpdates() {
     return tPermUpdate.getRoleChanges().values();
   }
 
-  public Collection<TPrivilegeChanges> getPrivilegeUpdates() {
+  Collection<TPrivilegeChanges> getPrivilegeUpdates() {
     return tPermUpdate.getPrivilegeChanges().values();
   }
 
-  public TPermissionsUpdate toThrift() {
+  TPermissionsUpdate toThrift() {
     return tPermUpdate;
   }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
index 7304fd8..49f39b1 100644
--- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
+++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
@@ -46,13 +46,12 @@ class SentryUpdater {
       }
     }
     try {
-      SentryAuthzUpdate sentryUpdates = sentryClient.getAllUpdatesFrom(
+      return sentryClient.getAllUpdatesFrom(
           authzInfo.getAuthzPermissions().getLastUpdatedSeqNum() + 1,
           authzInfo.getAuthzPaths().getLastUpdatedSeqNum() + 1);
-      return sentryUpdates;
     } catch (Exception e)  {
       sentryClient = null;
-      LOG.error("Error receiving updates from Sentry !!", e);
+      LOG.error("Error receiving updates from Sentry", e);
       return null;
     }
   }

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
index b8542b3..f4086fb 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -17,9 +17,7 @@
  */
 package org.apache.sentry.hdfs;
 
-import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
@@ -44,7 +42,7 @@ class DBUpdateForwarder<K extends Updateable.Update> {
   private static final Logger LOGGER = LoggerFactory.getLogger(DBUpdateForwarder.class);
 
   DBUpdateForwarder(final ImageRetriever<K> imageRetriever,
-      final DeltaRetriever<K> deltaRetriever) {
+                    final DeltaRetriever<K> deltaRetriever) {
     this.imageRetriever = imageRetriever;
     this.deltaRetriever = deltaRetriever;
   }
@@ -59,30 +57,26 @@ class DBUpdateForwarder<K extends Updateable.Update> {
    * @param seqNum the requested sequence number
    * @return a list of delta updates, e.g. {@link PathsUpdate} or {@link PermissionsUpdate}
    */
-   List<K> getAllUpdatesFrom(long seqNum) throws Exception {
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("#### GetAllUpdatesFrom [reqSeqNum = {} ]", seqNum);
-    }
-
-    // No newer updates available than the requested one.
+  List<K> getAllUpdatesFrom(long seqNum) throws Exception {
     long curSeqNum = deltaRetriever.getLatestDeltaID();
+    LOGGER.debug("GetAllUpdatesFrom sequence number {}, current sequence number is {}",
+            seqNum, curSeqNum);
     if (seqNum > curSeqNum) {
+      // No new deltas requested
       return Collections.emptyList();
     }
 
-    // Checks if there is newer deltas exists in the persistent storage.
-    // If there is, returns a list of delta updates.
+    // Checks if newer deltas exist in the persistent storage.
+    // If there are, return the list of delta updates.
     if ((seqNum != SentryStore.INIT_CHANGE_ID) &&
-          deltaRetriever.isDeltaAvailable(seqNum)) {
-      Collection<K> deltas = deltaRetriever.retrieveDelta(seqNum);
+            deltaRetriever.isDeltaAvailable(seqNum)) {
+      List<K> deltas = deltaRetriever.retrieveDelta(seqNum);
       if (!deltas.isEmpty()) {
-        return new LinkedList<>(deltas);
+        return deltas;
       }
     }
 
-    // Otherwise, a complete snapshot will be returned.
-    List<K> retVal = new LinkedList<>();
-    retVal.add(imageRetriever.retrieveFullImage());
-    return retVal;
+    // Return the full snapshot
+    return Collections.singletonList(imageRetriever.retrieveFullImage());
   }
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
index 5425114..fda7455 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathDeltaRetriever.java
@@ -17,21 +17,22 @@
  */
 package org.apache.sentry.hdfs;
 
-import com.codahale.metrics.Timer;
+import com.codahale.metrics.Timer.Context;
 import org.apache.sentry.provider.db.service.model.MSentryPathChange;
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
 
 import javax.annotation.concurrent.ThreadSafe;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 
 /**
  * PathDeltaRetriever retrieves delta updates of Hive Paths from a persistent
- * storage and translates them into a collection of {@code PathsUpdate} that the
+ * storage.
+ * Paths are translated into a collection of {@code PathsUpdate} that the
  * consumers, such as HDFS NameNode, can understand.
  * <p>
- * It is a thread safe class, as all the underlying database operation is thread safe.
+ * It is a thread safe class, as all the underlying database operation are thread safe.
  */
 @ThreadSafe
 public class PathDeltaRetriever implements DeltaRetriever<PathsUpdate> {
@@ -43,10 +44,10 @@ public class PathDeltaRetriever implements DeltaRetriever<PathsUpdate>
{
   }
 
   @Override
-  public Collection<PathsUpdate> retrieveDelta(long seqNum) throws Exception {
-    try (final Timer.Context timerContext =
+  public List<PathsUpdate> retrieveDelta(long seqNum) throws Exception {
+    try (final Context timerContext =
                  SentryHdfsMetricsUtil.getDeltaPathChangesTimer.time()) {
-      Collection<MSentryPathChange> mSentryPathChanges =
+      List<MSentryPathChange> mSentryPathChanges =
               sentryStore.getMSentryPathChanges(seqNum);
 
       SentryHdfsMetricsUtil.getDeltaPathChangesHistogram.update(mSentryPathChanges.size());
@@ -55,7 +56,7 @@ public class PathDeltaRetriever implements DeltaRetriever<PathsUpdate>
{
         return Collections.emptyList();
       }
 
-      Collection<PathsUpdate> updates = new ArrayList<>(mSentryPathChanges.size());
+      List<PathsUpdate> updates = new ArrayList<>(mSentryPathChanges.size());
       for (MSentryPathChange mSentryPathChange : mSentryPathChanges) {
         // Gets the changeID from the persisted MSentryPathChange.
         long changeID = mSentryPathChange.getChangeID();

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
index a29fc74..df6a0b0 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java
@@ -17,7 +17,7 @@
  */
 package org.apache.sentry.hdfs;
 
-import com.codahale.metrics.Timer;
+import com.codahale.metrics.Timer.Context;
 import org.apache.sentry.provider.db.service.model.MSentryPermChange;
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
 
@@ -25,6 +25,7 @@ import javax.annotation.concurrent.ThreadSafe;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 
 /**
  * PermDeltaRetriever retrieves delta updates of Sentry permission from a persistent
@@ -43,8 +44,8 @@ public class PermDeltaRetriever implements DeltaRetriever<PermissionsUpdate>
{
   }
 
   @Override
-  public Collection<PermissionsUpdate> retrieveDelta(long seqNum) throws Exception
{
-    try (final Timer.Context timerContext =
+  public List<PermissionsUpdate> retrieveDelta(long seqNum) throws Exception {
+    try (final Context timerContext =
                  SentryHdfsMetricsUtil.getDeltaPermChangesTimer.time()) {
       Collection<MSentryPermChange> mSentryPermChanges =
               sentryStore.getMSentryPermChanges(seqNum);
@@ -55,7 +56,7 @@ public class PermDeltaRetriever implements DeltaRetriever<PermissionsUpdate>
{
         return Collections.emptyList();
       }
 
-      Collection<PermissionsUpdate> updates = new ArrayList<>(mSentryPermChanges.size());
+      List<PermissionsUpdate> updates = new ArrayList<>(mSentryPermChanges.size());
       for (MSentryPermChange mSentryPermChange : mSentryPermChanges) {
         // Get the changeID from the persisted MSentryPermChange
         long changeID = mSentryPermChange.getChangeID();

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
index 5964f17..bad1099 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
@@ -17,7 +17,7 @@
  */
 package org.apache.sentry.hdfs;
 
-import com.codahale.metrics.Timer;
+import com.codahale.metrics.Timer.Context;
 import org.apache.sentry.hdfs.service.thrift.TPermissionsUpdate;
 import org.apache.sentry.hdfs.service.thrift.TPrivilegeChanges;
 import org.apache.sentry.hdfs.service.thrift.TRoleChanges;
@@ -25,8 +25,8 @@ import org.apache.sentry.provider.db.service.persistent.PermissionsImage;
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
 
 import javax.annotation.concurrent.ThreadSafe;
+import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
@@ -48,7 +48,7 @@ public class PermImageRetriever implements ImageRetriever<PermissionsUpdate>
{
 
   @Override
   public PermissionsUpdate retrieveFullImage() throws Exception {
-    try(Timer.Context timerContext =
+    try(Context timerContext =
         SentryHdfsMetricsUtil.getRetrievePermFullImageTimer.time()) {
 
       // Read the most up-to-date snapshot of Sentry perm information,
@@ -80,7 +80,7 @@ public class PermImageRetriever implements ImageRetriever<PermissionsUpdate>
{
         String role = privEnt.getKey();
         List<String> groups = privEnt.getValue();
         tPermUpdate.putToRoleChanges(role, new TRoleChanges(role, groups,
-            new LinkedList<String>()));
+            new ArrayList<String>()));
       }
 
       PermissionsUpdate permissionsUpdate = new PermissionsUpdate(tPermUpdate);

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
index 49d2360..b6b78bc 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
@@ -17,9 +17,25 @@
  */
 package org.apache.sentry.hdfs;
 
+import org.apache.sentry.core.common.exception.SentryHdfsServiceException;
+
+/**
+ * Private interface between HDFS NameNode and the Sentry Server.
+ * The only exposed service is update exchange via {@link #getAllUpdatesFrom(long, long)}
+ */
 public interface SentryHDFSServiceClient extends AutoCloseable {
+  /** Service name for Thrift */
   String SENTRY_HDFS_SERVICE_NAME = "SentryHDFSService";
 
-  SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum);
+  /**
+   * Get any permission and path updates accumulated since given sequence numbers.
+   * May return full update.
+   * @param permSeqNum Last sequence number for permissions update processed by the NameNode
plugin
+   * @param pathSeqNum Last sequence number for paths update processed by the NameNode plugin
+   * @return List of permission and path changes which may include a full snapshot.
+   * @throws SentryHdfsServiceException if a connection exception happens
+   */
+  SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum)
+          throws SentryHdfsServiceException;
 }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
index fc8bf4f..86d0f62 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
@@ -33,15 +33,16 @@ import org.apache.thrift.protocol.TCompactProtocol;
 import org.apache.thrift.protocol.TMultiplexedProtocol;
 import org.apache.thrift.protocol.TProtocol;
 
-import java.io.IOException;
-import java.util.LinkedList;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 
 import static org.apache.sentry.hdfs.service.thrift.sentry_hdfs_serviceConstants.UNUSED_PATH_UPDATE_IMG_NUM;
 
 /**
  * Sentry HDFS Service Client
  * <p>
- * The class isn't thread-safe - it is up to the aller to ensure thread safety
+ * The class isn't thread-safe - it is up to the caller to ensure thread safety
  */
 public class SentryHDFSServiceClientDefaultImpl
         implements SentryHDFSServiceClient, SentryConnection {
@@ -52,7 +53,7 @@ public class SentryHDFSServiceClientDefaultImpl
   private final long maxMessageSize;
 
   SentryHDFSServiceClientDefaultImpl(Configuration conf,
-                                     SentryTransportPool transportPool) throws IOException
{
+                                     SentryTransportPool transportPool) {
     maxMessageSize = conf.getLong(ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE,
             ClientConfig.SENTRY_HDFS_THRIFT_MAX_MESSAGE_SIZE_DEFAULT);
     useCompactTransport = conf.getBoolean(ClientConfig.USE_COMPACT_TRANSPORT,
@@ -76,7 +77,8 @@ public class SentryHDFSServiceClientDefaultImpl
     if (useCompactTransport) {
       tProtocol = new TCompactProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize);
     } else {
-      tProtocol = new TBinaryProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize,
true, true);
+      tProtocol = new TBinaryProtocol(transport.getTTransport(), maxMessageSize, maxMessageSize,
+              true, true);
     }
     TMultiplexedProtocol protocol = new TMultiplexedProtocol(
             tProtocol, SentryHDFSServiceClient.SENTRY_HDFS_SERVICE_NAME);
@@ -87,24 +89,30 @@ public class SentryHDFSServiceClientDefaultImpl
   @Override
   public SentryAuthzUpdate getAllUpdatesFrom(long permSeqNum, long pathSeqNum)
           throws SentryHdfsServiceException {
-    SentryAuthzUpdate retVal = new SentryAuthzUpdate(new LinkedList<PermissionsUpdate>(),
new LinkedList<PathsUpdate>());
     try {
-      TAuthzUpdateRequest updateRequest = new TAuthzUpdateRequest(permSeqNum, pathSeqNum,
UNUSED_PATH_UPDATE_IMG_NUM);
+      TAuthzUpdateRequest updateRequest = new TAuthzUpdateRequest(permSeqNum, pathSeqNum,
+              UNUSED_PATH_UPDATE_IMG_NUM);
       TAuthzUpdateResponse sentryUpdates = client.get_authz_updates(updateRequest);
+      List<PathsUpdate> pathsUpdates = Collections.emptyList();
       if (sentryUpdates.getAuthzPathUpdate() != null) {
+        pathsUpdates = new ArrayList<>(sentryUpdates.getAuthzPathUpdate().size());
         for (TPathsUpdate pathsUpdate : sentryUpdates.getAuthzPathUpdate()) {
-          retVal.getPathUpdates().add(new PathsUpdate(pathsUpdate));
+          pathsUpdates.add(new PathsUpdate(pathsUpdate));
         }
       }
+
+      List<PermissionsUpdate> permsUpdates = Collections.emptyList();
       if (sentryUpdates.getAuthzPermUpdate() != null) {
+        permsUpdates = new ArrayList<>(sentryUpdates.getAuthzPermUpdate().size());
         for (TPermissionsUpdate permsUpdate : sentryUpdates.getAuthzPermUpdate()) {
-          retVal.getPermUpdates().add(new PermissionsUpdate(permsUpdate));
+          permsUpdates.add(new PermissionsUpdate(permsUpdate));
         }
       }
+
+      return new SentryAuthzUpdate(permsUpdates, pathsUpdates);
     } catch (Exception e) {
       throw new SentryHdfsServiceException("Thrift Exception occurred !!", e);
     }
-    return retVal;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
index 28d6f5b..1c7061b 100644
--- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
+++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
@@ -18,11 +18,12 @@
 
 package org.apache.sentry.hdfs;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
-import com.codahale.metrics.Timer;
+import com.codahale.metrics.Timer.Context;
 import org.apache.sentry.hdfs.service.thrift.SentryHDFSService;
 import org.apache.sentry.hdfs.service.thrift.TAuthzUpdateRequest;
 import org.apache.sentry.hdfs.service.thrift.TAuthzUpdateResponse;
@@ -34,8 +35,11 @@ import org.slf4j.LoggerFactory;
 
 import static org.apache.sentry.hdfs.service.thrift.sentry_hdfs_serviceConstants.UNUSED_PATH_UPDATE_IMG_NUM;
 
+/**
+ * Process requests from HDFS Name Node plugin.
+ * The only supported request is {@link #get_all_authz_updates_from(long, long)}.
+ */
 public class SentryHDFSServiceProcessor implements SentryHDFSService.Iface {
-
   private static final Logger LOGGER = LoggerFactory.getLogger(SentryHDFSServiceProcessor.class);
 
   @Override
@@ -47,55 +51,57 @@ public class SentryHDFSServiceProcessor implements SentryHDFSService.Iface
{
   public TAuthzUpdateResponse get_authz_updates(TAuthzUpdateRequest request)
       throws TException {
     TAuthzUpdateResponse retVal = new TAuthzUpdateResponse();
-    retVal.setAuthzPathUpdate(new LinkedList<TPathsUpdate>());
-    retVal.setAuthzPermUpdate(new LinkedList<TPermissionsUpdate>());
-    if (SentryPlugin.instance != null) {
-      final Timer.Context timerContext =
-          SentryHdfsMetricsUtil.getAllAuthzUpdatesTimer.time();
-      try {
-        List<PermissionsUpdate> permUpdates =
-            SentryPlugin.instance.getAllPermsUpdatesFrom(request.getPermSeqNum());
-        SentryHdfsMetricsUtil.getPermUpdateHistogram.update(permUpdates.size());
-        List<PathsUpdate> pathUpdates =
-            SentryPlugin.instance.getAllPathsUpdatesFrom(request.getPathSeqNum());
-        SentryHdfsMetricsUtil.getPathUpdateHistogram.update(pathUpdates.size());
-        for (PathsUpdate update : pathUpdates) {
-          if (LOGGER.isDebugEnabled()) {
-            LOGGER.debug("### Sending PATH preUpdate seq [" + update.getSeqNum() + "] ###");
-            LOGGER.debug("### Sending PATH preUpdate [" + update.toThrift() + "] ###");
-          }
-          retVal.getAuthzPathUpdate().add(update.toThrift());
-        }
-        for (PermissionsUpdate update : permUpdates) {
-          if (LOGGER.isDebugEnabled()) {
-            LOGGER.debug("### Sending PERM preUpdate seq [" + update.getSeqNum() + "] ###");
-            LOGGER.debug("### Sending PERM preUpdate [" + update.toThrift() + "] ###");
-          }
-          retVal.getAuthzPermUpdate().add(update.toThrift());
+
+    if (SentryPlugin.instance == null) {
+      LOGGER.error("SentryPlugin not initialized yet !!");
+      retVal.setAuthzPathUpdate(Collections.<TPathsUpdate>emptyList());
+      retVal.setAuthzPermUpdate(Collections.<TPermissionsUpdate>emptyList());
+      return retVal;
+    }
+
+    try (Context timerContext =
+                 SentryHdfsMetricsUtil.getAllAuthzUpdatesTimer.time()){
+      List<PermissionsUpdate> permUpdates =
+          SentryPlugin.instance.getAllPermsUpdatesFrom(request.getPermSeqNum());
+      SentryHdfsMetricsUtil.getPermUpdateHistogram.update(permUpdates.size());
+      List<PathsUpdate> pathUpdates =
+          SentryPlugin.instance.getAllPathsUpdatesFrom(request.getPathSeqNum());
+      SentryHdfsMetricsUtil.getPathUpdateHistogram.update(pathUpdates.size());
+
+      List<TPathsUpdate> retPathUpdates = new ArrayList<>(pathUpdates.size());
+      for (PathsUpdate update : pathUpdates) {
+        LOGGER.debug("Sending PATH preUpdate seq [{}], [{}]",
+                update.getSeqNum(), update.toThrift());
+        retPathUpdates.add(update.toThrift());
+      }
+      retVal.setAuthzPathUpdate(retPathUpdates);
+
+      List<TPermissionsUpdate>retPermUpdates = new ArrayList<>(permUpdates.size());
+      for (PermissionsUpdate update : permUpdates) {
+        LOGGER.debug("Sending PERM preUpdate seq [{}], [{}]",
+                update.getSeqNum(), update.toThrift());
+        retPermUpdates.add(update.toThrift());
+      }
+      retVal.setAuthzPermUpdate(retPermUpdates);
+
+      if (LOGGER.isDebugEnabled()) {
+        StringBuilder permSeq = new StringBuilder("<");
+        for (PermissionsUpdate permUpdate : permUpdates) {
+          permSeq.append(permUpdate.getSeqNum()).append(",");
         }
-        if (LOGGER.isDebugEnabled()) {
-          StringBuilder permSeq = new StringBuilder("<");
-          for (PermissionsUpdate permUpdate : permUpdates) {
-            permSeq.append(permUpdate.getSeqNum()).append(",");
-          }
-          permSeq.append(">");
-          StringBuilder pathSeq = new StringBuilder("<");
-          for (PathsUpdate pathUpdate : pathUpdates) {
-            pathSeq.append(pathUpdate.getSeqNum()).append(",");
-          }
-          pathSeq.append(">");
-          LOGGER.debug("#### Updates requested from HDFS ["
-              + "permReq=" + request.getPermSeqNum() + ", permResp=" + permSeq + "] "
-              + "[pathReq=" + request.getPathSeqNum() + ", pathResp=" + pathSeq + "]");
+        permSeq.append(">");
+        StringBuilder pathSeq = new StringBuilder("<");
+        for (PathsUpdate pathUpdate : pathUpdates) {
+          pathSeq.append(pathUpdate.getSeqNum()).append(",");
         }
-      } catch (Exception e) {
-        LOGGER.error("Error Sending updates to downstream Cache", e);
-        throw new TException(e);
-      } finally {
-        timerContext.stop();
+        pathSeq.append(">");
+        LOGGER.debug("Updates requested from HDFS ["
+            + "permReq=" + request.getPermSeqNum() + ", permResp=" + permSeq + "] "
+            + "[pathReq=" + request.getPathSeqNum() + ", pathResp=" + pathSeq + "]");
       }
-    } else {
-      LOGGER.error("SentryPlugin not initialized yet !!");
+    } catch (Exception e) {
+      LOGGER.error("Error Sending updates to downstream Cache", e);
+      throw new TException(e);
     }
 
     return retVal;

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
index f51894b..5471a02 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
@@ -19,6 +19,7 @@
 package org.apache.sentry.provider.db.service.model;
 
 import javax.jdo.annotations.PersistenceCapable;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -33,9 +34,9 @@ public class MAuthzPathsMapping {
   private Set<MPath> paths;
   private long createTimeMs;
 
-  public MAuthzPathsMapping(String authzObjName, Iterable<String> paths) {
+  public MAuthzPathsMapping(String authzObjName, Collection<String> paths) {
     this.authzObjName = MSentryUtil.safeIntern(authzObjName);
-    this.paths = new HashSet<>();
+    this.paths = new HashSet<>(paths.size());
     for (String path : paths) {
       this.paths.add(new MPath(path));
     }

http://git-wip-us.apache.org/repos/asf/sentry/blob/a785d29c/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 8b19c88..f088f5c 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
@@ -27,7 +27,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -2489,7 +2488,7 @@ public class SentryStore {
       for (MSentryRole role : mGroup.getRoles()) {
         List<String> rUpdate = retVal.get(role.getRoleName());
         if (rUpdate == null) {
-          rUpdate = new LinkedList<>();
+          rUpdate = new ArrayList<>();
           retVal.put(role.getRoleName(), rUpdate);
         }
         rUpdate.add(mGroup.getGroupName());
@@ -2594,7 +2593,7 @@ public class SentryStore {
    * @param update the corresponding path delta update
    * @throws Exception
    */
-  public void addAuthzPathsMapping(final String authzObj, final Iterable<String> paths,
+  public void addAuthzPathsMapping(final String authzObj, final Collection<String>
paths,
       final Update update) throws Exception {
     execute(new DeltaTransactionBlock(update), new TransactionBlock<Object>() {
       public Object execute(PersistenceManager pm) throws Exception {
@@ -2615,7 +2614,7 @@ public class SentryStore {
    * @param paths a set of paths need to be added into the authzObj -> [Paths] mapping
    */
   private void addAuthzPathsMappingCore(PersistenceManager pm, String authzObj,
-        Iterable<String> paths) {
+        Collection<String> paths) {
     MAuthzPathsMapping mAuthzPathsMapping = getMAuthzPathsMappingCore(pm, authzObj);
     if (mAuthzPathsMapping == null) {
       mAuthzPathsMapping = new MAuthzPathsMapping(authzObj, paths);
@@ -3303,7 +3302,7 @@ public class SentryStore {
         for (String roleName : roleNames) {
           Set<TSentryGroup> tSentryGroups = roleGroupsMap.get(roleName);
           if (tSentryGroups == null) {
-            tSentryGroups = Sets.newHashSet();
+            tSentryGroups = new HashSet<>();
           }
           tSentryGroups.add(new TSentryGroup(entry.getKey()));
           roleGroupsMap.put(roleName, tSentryGroups);


Mime
View raw message