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 <[email protected]> Authored: Tue Jun 20 16:48:57 2017 -0700 Committer: Alexander Kolbasov <[email protected]> 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);
