You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda <kk...@cloudera.com> on 2017/03/24 00:19:35 UTC
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/
-----------------------------------------------------------
(Updated March 24, 2017, 12:19 a.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Changed the summary
Summary (updated)
-----------------
SENTRY-1593 Implement client failover for all the sentry clients
Bugs: SENTRY-1593
https://issues.apache.org/jira/browse/SENTRY-1593
Repository: sentry
Description
-------
There are couple of things done as part of this change set
1. Extended the capability of sentry Namenode client and generic policy client to connect to multiple servers and failover to the server that is available
2. Made design change so that logic that handles the transport layer of the client to another class so that we don't have to duplicate the code and also more maintainable.
3. Moved the service constants need to handle the transport of these clients separately.
Diffs
-----
sentry-core/sentry-core-common/pom.xml 9d18063
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
Diff: https://reviews.apache.org/r/57901/diff/1/
Testing
-------
Thanks,
kalyan kumar kalvagadda
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
> On March 31, 2017, 6:18 a.m., Alexander Kolbasov wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
> > Lines 33 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680343#file1680343line33>
> >
> > I think this should be AutoCloseable
>
> kalyan kumar kalvagadda wrote:
> Why not Closable?
Comment from http://stackoverflow.com/questions/13141302/implements-closeable-or-implements-autocloseable. Seems like a very usuable thing for clients.
Implementing AutoCloseable allows a class to be used as a resource of the try-with-resources construct introduced in Java 7, which allows closing such resources automatically at the end of a block, without having to add a finally block which closes the resource explicitely.
- Vamsee
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170671
-----------------------------------------------------------
On April 18, 2017, 11:18 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 11:18 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d180635ca5ed6fb1def4ffdc4addda5a650593e
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/MissingConfigurationException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransport.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java 4ed1361fb1d79620c3098355a0ea9c27203a75de
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2512902a8500bfacb1c745ca4b10498cc8
> sentry-hdfs/sentry-hdfs-dist/pom.xml beda2029616538101dc368b3d272fdfc942868ad
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 085971b34e3901b7a1d59bd8e7516b25f81ca872
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf402a9a04b28e2c6c06d4e55a3607132ead
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 03bf39e1bed10dcd706ef12c078305b6497874f4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b1524546840df9e3275bd8a8220cc9de1b1f
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2bb6ec748a42ae2b65e937f6dec492fc48
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3172533fa768145d23664ab536dcadd6b3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81bd657e00841f4c8bb63a8851d1a44f8b78
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0fdb827bfcf33814a3ffa56c1898edd2521
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ee6cdf79f0cc96a5a72de8d79ed9c175e9aae9a3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d9306d848d54fd476252793afc4c3a7fc6a08
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a522217bc26d768026f1c6be8c3e758fad8
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a4d32f6c5588b277c637e041ca8b3d999e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a16a26939f4cf68278c7b8204ec0b08ca7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5d62b88bd5c2f696af8f8872c42c02de9c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 2cf748e1e538c99fc34c57ed50b78701977c84ad
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a4662c1c564dda8d050340e6b06cdce516530
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a98f1bdb5bc9bd6c2c86b6fa40413b94bca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04ad079869c77d7b60085db6ba7791f586ab
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb56d05dc699634927106f6fa31e74fa213
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java c4964c34f6714c96d578bed49efc68d7f13e5925
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23232f887b9c00662f7cef01e1a72c56eb7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497d9fdc8536d4bb9f52fa3d0f69acf78908
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9696d12cf353b59aa9b877f7ee971caf25
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf4359c0d057f441ddd1f738f5468eaee5e385
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec884041b361df90e7186f05d0c964e1ba89fc4
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5abaf6f3e52ca7d0a3a7862b0d39b6445016
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92ddc56932e49e555c88136255ce234738dbe
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79aeb998d32f1f50e14328977ea1946d13029
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 729238707b9b78776e9db996de104c8f13f843bf
>
>
> Diff: https://reviews.apache.org/r/57901/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On March 31, 2017, 6:18 a.m., Alexander Kolbasov wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
> > Lines 33 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680343#file1680343line33>
> >
> > I think this should be AutoCloseable
Why not Closable?
> On March 31, 2017, 6:18 a.m., Alexander Kolbasov wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
> > Lines 49 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680343#file1680343line49>
> >
> > This is wrong - you are redefining close() from the parent while changing exceptions thrown - you should just remove this line
Closing of client is needed to close the underneath socket. socket close doesn't throw any exception. That is why I have re-defined the close which doesn't throw an exeption.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170671
-----------------------------------------------------------
On April 18, 2017, 11:18 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 11:18 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d180635ca5ed6fb1def4ffdc4addda5a650593e
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/MissingConfigurationException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransport.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java 4ed1361fb1d79620c3098355a0ea9c27203a75de
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2512902a8500bfacb1c745ca4b10498cc8
> sentry-hdfs/sentry-hdfs-dist/pom.xml beda2029616538101dc368b3d272fdfc942868ad
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 085971b34e3901b7a1d59bd8e7516b25f81ca872
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf402a9a04b28e2c6c06d4e55a3607132ead
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 03bf39e1bed10dcd706ef12c078305b6497874f4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b1524546840df9e3275bd8a8220cc9de1b1f
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2bb6ec748a42ae2b65e937f6dec492fc48
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3172533fa768145d23664ab536dcadd6b3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81bd657e00841f4c8bb63a8851d1a44f8b78
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0fdb827bfcf33814a3ffa56c1898edd2521
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ee6cdf79f0cc96a5a72de8d79ed9c175e9aae9a3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d9306d848d54fd476252793afc4c3a7fc6a08
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a522217bc26d768026f1c6be8c3e758fad8
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a4d32f6c5588b277c637e041ca8b3d999e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a16a26939f4cf68278c7b8204ec0b08ca7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5d62b88bd5c2f696af8f8872c42c02de9c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 2cf748e1e538c99fc34c57ed50b78701977c84ad
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a4662c1c564dda8d050340e6b06cdce516530
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a98f1bdb5bc9bd6c2c86b6fa40413b94bca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04ad079869c77d7b60085db6ba7791f586ab
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb56d05dc699634927106f6fa31e74fa213
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java c4964c34f6714c96d578bed49efc68d7f13e5925
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23232f887b9c00662f7cef01e1a72c56eb7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497d9fdc8536d4bb9f52fa3d0f69acf78908
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9696d12cf353b59aa9b877f7ee971caf25
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf4359c0d057f441ddd1f738f5468eaee5e385
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec884041b361df90e7186f05d0c964e1ba89fc4
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5abaf6f3e52ca7d0a3a7862b0d39b6445016
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92ddc56932e49e555c88136255ce234738dbe
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79aeb998d32f1f50e14328977ea1946d13029
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 729238707b9b78776e9db996de104c8f13f843bf
>
>
> Diff: https://reviews.apache.org/r/57901/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170671
-----------------------------------------------------------
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
Lines 32 (patched)
<https://reviews.apache.org/r/57901/#comment243560>
I don't think this is the right interface and the right name for it.
What are you trying to do? What is the primitive operation and its meaning?
You want something that can connect. The fact that it does it with some retries is its implementation detail which is irrelevant for the interface.
You want to say - hey - stop using the connection (close() or something similar)
That's it.
And getRetryCount() doesn't belong here at all.
So I think your interface should be something like this:
interface Connector extends AutoCloseable {
void connect() throws something;
}
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
Lines 33 (patched)
<https://reviews.apache.org/r/57901/#comment243561>
I think this should be AutoCloseable
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java
Lines 49 (patched)
<https://reviews.apache.org/r/57901/#comment243562>
This is wrong - you are redefining close() from the parent while changing exceptions thrown - you should just remove this line
- Alexander Kolbasov
On March 29, 2017, 11:32 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated March 29, 2017, 11:32 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> There are couple of things done as part of this change set
> 1. Extended the capability of sentry Namenode client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Made design change so that logic that handles the transport layer of the client to another class so that we don't have to duplicate the code and also more maintainable.
> 3. Moved the service constants need to handle the transport of these clients separately.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On March 30, 2017, 11:59 p.m., Alexander Kolbasov wrote:
> > I think that this should be split into two parts:
> >
> > 1) Move files around without changing them (except as needed for the mvoe)
> > 2) Actually changing files
> >
> > The first one should be very straightforward and the second one will concentrate on actual HA work. This would be much easier to review.
Agree, I will be splitting the review into two parts and posting them again.
> On March 30, 2017, 11:59 p.m., Alexander Kolbasov wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
> > Lines 21 (patched)
> > <https://reviews.apache.org/r/57901/diff/1/?file=1673568#file1673568line21>
> >
> > Please add javadoc for the class
This was existing file. I just moved it to another location. I will anyway add javadoc for this exception.
> On March 30, 2017, 11:59 p.m., Alexander Kolbasov wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
> > Lines 28 (patched)
> > <https://reviews.apache.org/r/57901/diff/1/?file=1673568#file1673568line28>
> >
> > This constructor is never used
This was existing file. I just moved it to another location. As a convention most of the Exception classes have these constrcutors defined. If not now, they can be used later.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170070
-----------------------------------------------------------
On March 31, 2017, 10:28 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 10:28 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170070
-----------------------------------------------------------
I think that this should be split into two parts:
1) Move files around without changing them (except as needed for the mvoe)
2) Actually changing files
The first one should be very straightforward and the second one will concentrate on actual HA work. This would be much easier to review.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
Lines 21 (patched)
<https://reviews.apache.org/r/57901/#comment242858>
Please add javadoc for the class
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
Lines 28 (patched)
<https://reviews.apache.org/r/57901/#comment242857>
This constructor is never used
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 22 (patched)
<https://reviews.apache.org/r/57901/#comment243520>
Please do not comment out imports, remove them instead.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 62 (patched)
<https://reviews.apache.org/r/57901/#comment243521>
Our convention is to use clientObject rather then client_object
- Alexander Kolbasov
On March 29, 2017, 11:32 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated March 29, 2017, 11:32 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> There are couple of things done as part of this change set
> 1. Extended the capability of sentry Namenode client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Made design change so that logic that handles the transport layer of the client to another class so that we don't have to duplicate the code and also more maintainable.
> 3. Moved the service constants need to handle the transport of these clients separately.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On April 6, 2017, 1:58 p.m., Na Li wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
> > Lines 86 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680337#file1680337line86>
> >
> > At line 64, you did not check if client_object is null or not. Is it acceptable when this input is null? When you call it at line 86 and if client is null, will it cause exception and terminate the retry?
If the client construction fails exception is thrown and RetryClientInvocationHandler will not be constructed at all.
> On April 6, 2017, 1:58 p.m., Na Li wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
> > Lines 207 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680344#file1680344line207>
> >
> > should serverAddress be added into endpoints even when there is only one server?
To make the logic uniform I have removed serverAddress member from the class.
> On April 6, 2017, 1:58 p.m., Na Li wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
> > Lines 249 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680344#file1680344line249>
> >
> > line 207 set endpoints as null. so null exception could happen here.
o make the logic uniform I have removed serverAddress member from the class and endpoints is populated always.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170725
-----------------------------------------------------------
On April 18, 2017, 11:18 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 11:18 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d180635ca5ed6fb1def4ffdc4addda5a650593e
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/MissingConfigurationException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransport.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java 4ed1361fb1d79620c3098355a0ea9c27203a75de
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2512902a8500bfacb1c745ca4b10498cc8
> sentry-hdfs/sentry-hdfs-dist/pom.xml beda2029616538101dc368b3d272fdfc942868ad
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 085971b34e3901b7a1d59bd8e7516b25f81ca872
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf402a9a04b28e2c6c06d4e55a3607132ead
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 03bf39e1bed10dcd706ef12c078305b6497874f4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b1524546840df9e3275bd8a8220cc9de1b1f
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2bb6ec748a42ae2b65e937f6dec492fc48
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3172533fa768145d23664ab536dcadd6b3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81bd657e00841f4c8bb63a8851d1a44f8b78
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0fdb827bfcf33814a3ffa56c1898edd2521
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ee6cdf79f0cc96a5a72de8d79ed9c175e9aae9a3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d9306d848d54fd476252793afc4c3a7fc6a08
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a522217bc26d768026f1c6be8c3e758fad8
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a4d32f6c5588b277c637e041ca8b3d999e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a16a26939f4cf68278c7b8204ec0b08ca7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5d62b88bd5c2f696af8f8872c42c02de9c
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 2cf748e1e538c99fc34c57ed50b78701977c84ad
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a4662c1c564dda8d050340e6b06cdce516530
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a98f1bdb5bc9bd6c2c86b6fa40413b94bca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04ad079869c77d7b60085db6ba7791f586ab
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb56d05dc699634927106f6fa31e74fa213
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java c4964c34f6714c96d578bed49efc68d7f13e5925
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23232f887b9c00662f7cef01e1a72c56eb7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497d9fdc8536d4bb9f52fa3d0f69acf78908
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9696d12cf353b59aa9b877f7ee971caf25
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf4359c0d057f441ddd1f738f5468eaee5e385
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec884041b361df90e7186f05d0c964e1ba89fc4
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5abaf6f3e52ca7d0a3a7862b0d39b6445016
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92ddc56932e49e555c88136255ce234738dbe
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79aeb998d32f1f50e14328977ea1946d13029
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 729238707b9b78776e9db996de104c8f13f843bf
>
>
> Diff: https://reviews.apache.org/r/57901/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170725
-----------------------------------------------------------
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 86 (patched)
<https://reviews.apache.org/r/57901/#comment243611>
At line 64, you did not check if client_object is null or not. Is it acceptable when this input is null? When you call it at line 86 and if client is null, will it cause exception and terminate the retry?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
Lines 207 (patched)
<https://reviews.apache.org/r/57901/#comment243628>
should serverAddress be added into endpoints even when there is only one server?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
Lines 249 (patched)
<https://reviews.apache.org/r/57901/#comment243635>
line 207 set endpoints as null. so null exception could happen here.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 63 (patched)
<https://reviews.apache.org/r/57901/#comment244085>
do we allow clientObject to be null? If not, we need to checkNotNull for clientObject
- Na Li
On April 3, 2017, 5:35 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated April 3, 2017, 5:35 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/
-----------------------------------------------------------
(Updated April 3, 2017, 5:35 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Addressed review comments.
Bugs: SENTRY-1593
https://issues.apache.org/jira/browse/SENTRY-1593
Repository: sentry
Description
-------
At a high level the change set submitted does following.
1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
1. RetryClientInvocationHandler.java
2. SentryClientInvocationHandler.java
3. ThriftUtil.java
4. SentryHdfsServiceException.java
Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
Diffs (updated)
-----
sentry-core/sentry-core-common/pom.xml 9d18063
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
Diff: https://reviews.apache.org/r/57901/diff/3/
Changes: https://reviews.apache.org/r/57901/diff/2-3/
Testing
-------
Thanks,
kalyan kumar kalvagadda
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/
-----------------------------------------------------------
(Updated March 31, 2017, 10:28 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Updated the Description. Please have look into it while reviewing the code.
Bugs: SENTRY-1593
https://issues.apache.org/jira/browse/SENTRY-1593
Repository: sentry
Description (updated)
-------
At a high level the change set submitted does following.
1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
1. RetryClientInvocationHandler.java
2. SentryClientInvocationHandler.java
3. ThriftUtil.java
4. SentryHdfsServiceException.java
Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
Diffs
-----
sentry-core/sentry-core-common/pom.xml 9d18063
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
Diff: https://reviews.apache.org/r/57901/diff/2/
Testing
-------
Thanks,
kalyan kumar kalvagadda
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
> > Lines 31 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680336#file1680336line31>
> >
> > Remove the blank lines.
Will fix
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
> > Lines 91 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680337#file1680337line91>
> >
> > Should we log the error?
When exception is thrown by connectWithRetry method, it is logged. Logging it here will be a duplicate lo again.
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
> > Lines 111 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680337#file1680337line111>
> >
> > Should use `LOGGER.warnning` or `LOGGER.error` to be more visible in production.
Will fix
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java
> > Lines 26 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680338#file1680338line26>
> >
> > Can we define this as `Closable` as well?
> >
> > So that it can be used in `try-resource` in JDK7.
Underneath client is already Closable. Do you still see any need for SentryClientInvocationHandler to be closable as well?
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
> > Lines 247 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680344#file1680344line247>
> >
> > `ConnectTo...` -> `connectTo...`
Will fix.
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
> > Lines 249 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680344#file1680344line249>
> >
> > Can this be handled in the loop?
> > And here the `serverAddress` is not set, would it be problematic?
Agree, I have seperated this from the loop for a reason which is not valid now. I will address it.
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
> > Lines 269 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680344#file1680344line269>
> >
> > What will be thrown if `endpoints.isEmpty()`?
endpoints will never be empty. If the configuration from which this ArrayList is constrcuted is empty, exception will be thrown and the client object will fail.
> On March 30, 2017, 10:01 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
> > Lines 32 (patched)
> > <https://reviews.apache.org/r/57901/diff/2/?file=1680346#file1680346line32>
> >
> > Please add some comments to this utility class.
This was existing file. I just moved it to another location. I will anyway add javadoc for this class.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170644
-----------------------------------------------------------
On March 31, 2017, 10:28 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated March 31, 2017, 10:28 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> At a high level the change set submitted does following.
> 1. Extends the capability of sentry name-node client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Transport handling of the clients is abstracted to another class so that the client's just have to extend the class that abstracts it. Implementations of the clients don't have to worry about it.
> 3. Changed the behavior of the clients so that they connect to one of the configured servers when the client is actually instantiated.
>
> As part of the code changes there are couple of files that are moved from one package to another. Here is the list of them
> 1. RetryClientInvocationHandler.java
> 2. SentryClientInvocationHandler.java
> 3. ThriftUtil.java
> 4. SentryHdfsServiceException.java
> Amount these files, RetryClientInvocationHandler.java has changed. Please have a closer look at this file.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by Lei Xu <le...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/#review170644
-----------------------------------------------------------
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
Lines 21 (patched)
<https://reviews.apache.org/r/57901/#comment243524>
Could you add more comments to it?
Btw, should it be a {{IOException}} or {{TException}}. {{RuntimeException}} is a unchecked exception, that is very easy to be ignored by the caller (i.e., IDE will not force you to add that to the function signature).
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java
Lines 31 (patched)
<https://reviews.apache.org/r/57901/#comment243525>
Remove the blank lines.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 91 (patched)
<https://reviews.apache.org/r/57901/#comment243530>
Should we log the error?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 111 (patched)
<https://reviews.apache.org/r/57901/#comment243533>
Should use `LOGGER.warnning` or `LOGGER.error` to be more visible in production.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Lines 116 (patched)
<https://reviews.apache.org/r/57901/#comment243532>
Can it directly `throw targetException` here?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java
Lines 26 (patched)
<https://reviews.apache.org/r/57901/#comment243528>
Can we define this as `Closable` as well?
So that it can be used in `try-resource` in JDK7.
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
Lines 247 (patched)
<https://reviews.apache.org/r/57901/#comment243534>
`ConnectTo...` -> `connectTo...`
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
Lines 249 (patched)
<https://reviews.apache.org/r/57901/#comment243536>
Can this be handled in the loop?
And here the `serverAddress` is not set, would it be problematic?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java
Lines 269 (patched)
<https://reviews.apache.org/r/57901/#comment243535>
What will be thrown if `endpoints.isEmpty()`?
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java
Lines 32 (patched)
<https://reviews.apache.org/r/57901/#comment243537>
Please add some comments to this utility class.
- Lei Xu
On March 29, 2017, 4:32 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57901/
> -----------------------------------------------------------
>
> (Updated March 29, 2017, 4:32 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1593
> https://issues.apache.org/jira/browse/SENTRY-1593
>
>
> Repository: sentry
>
>
> Description
> -------
>
> There are couple of things done as part of this change set
> 1. Extended the capability of sentry Namenode client and generic policy client to connect to multiple servers and failover to the server that is available
> 2. Made design change so that logic that handles the transport layer of the client to another class so that we don't have to duplicate the code and also more maintainable.
> 3. Moved the service constants need to handle the transport of these clients separately.
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/pom.xml 9d18063
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
>
>
> Diff: https://reviews.apache.org/r/57901/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 57901: SENTRY-1593 Implement client failover for
all the sentry clients
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57901/
-----------------------------------------------------------
(Updated March 29, 2017, 11:32 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Added code changes to fix the unit test failures and also added changes making SentryServiceClientTransportDefaultImpl.java more modular.
Bugs: SENTRY-1593
https://issues.apache.org/jira/browse/SENTRY-1593
Repository: sentry
Description
-------
There are couple of things done as part of this change set
1. Extended the capability of sentry Namenode client and generic policy client to connect to multiple servers and failover to the server that is available
2. Made design change so that logic that handles the transport layer of the client to another class so that we don't have to duplicate the code and also more maintainable.
3. Moved the service constants need to handle the transport of these clients separately.
Diffs (updated)
-----
sentry-core/sentry-core-common/pom.xml 9d18063
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryHdfsServiceException.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientInvocationHandler.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 6cea596
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 636de40
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 12175f7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java 038bca7
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClientTransportDefaultImpl.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java PRE-CREATION
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java PRE-CREATION
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 23552c2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java ab12bf4
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 28b1224
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java 2a18b15
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java 4dc99a2
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsServiceException.java 307d8c3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java 919b81b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorWrapper.java d320d0f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 075983e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 980d930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java f6bb8a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java fb7c40a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java 8cf1c1a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java c2b03e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 4284b53
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java ee2a466
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryProcessorWrapper.java a5f11a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ThriftUtil.java 5fed04a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java d5f4fcb
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/RetryClientInvocationHandler.java 2f38198
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryClientInvocationHandler.java b8c7f23
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java f822497
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java d3a68c9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java d4bf435
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java 1ec8840
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactoryGM.java dfae5ab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java 04d92dd
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java 7292387
Diff: https://reviews.apache.org/r/57901/diff/2/
Changes: https://reviews.apache.org/r/57901/diff/1-2/
Testing
-------
Thanks,
kalyan kumar kalvagadda