You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Alexander Kolbasov <ak...@gmail.com> on 2017/05/11 06:19:01 UTC

Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/1/


Testing
-------


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 16, 2017, 10:42 p.m., Sergio Pena wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 102 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line102>
> >
> >     You may use conf.getStrings() internally on transportConfig.getSentryServerRpcAddress(). This conf.getStrings() will return the command delimited values as an array.

It doesn't work - returns null if there is no comma


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175162
-----------------------------------------------------------


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 16, 2017, 10:42 p.m., Sergio Pena wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line94>
> >
> >     The class annotates is thread safe, but this incremental isn't, is it?
> >     
> >     poolId is static meaning it will be shared across threads.

poolId is just used for debugging for tests, so there is no need to make it thread-safe. I added a comment for it.


> On May 16, 2017, 10:42 p.m., Sergio Pena wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 117 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line117>
> >
> >     Should we move this before getting the list of server addresses? If pool is disabled, then the user might not have any host and port address, right?

The host and port is still needed - this is orthogonal for the pool vs no pool.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175162
-----------------------------------------------------------


On May 13, 2017, 10:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 10:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175162
-----------------------------------------------------------




sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 94 (patched)
<https://reviews.apache.org/r/59167/#comment248550>

    The class annotates is thread safe, but this incremental isn't, is it?
    
    poolId is static meaning it will be shared across threads.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 102 (patched)
<https://reviews.apache.org/r/59167/#comment248552>

    You may use conf.getStrings() internally on transportConfig.getSentryServerRpcAddress(). This conf.getStrings() will return the command delimited values as an array.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 117 (patched)
<https://reviews.apache.org/r/59167/#comment248554>

    Should we move this before getting the list of server addresses? If pool is disabled, then the user might not have any host and port address, right?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 127 (patched)
<https://reviews.apache.org/r/59167/#comment248555>

    typo: no tlimit


- Sergio Pena


On May 13, 2017, 10:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 10:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
> > Line 362 (original), 362 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717912#file1717912line362>
> >
> >     LOG.error(e) ?
> >     
> >     Btw, should it re-throw the exception?

This class is no longer used I just had to handle the exception to be able to compile.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
> > Line 395 (original), 393 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717912#file1717912line395>
> >
> >     LOG the Exception and/or rethrow it?

Same thing - this class is never used.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717914#file1717914line374>
> >
> >     Log.error(e) and/or re-throw it.

This class is never used.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 150 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line150>
> >
> >     What is the advantages of random pick against round-robin?

Round-robin requires keeping state between invocations and keeping state requires synchronization. The current mechhanism is stateless and doesn't require any locking.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 173 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line173>
> >
> >     If all addresses throw `NoSuchElementException`, will it throw a null in the end of the method?

This was added for debugging, removed.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 235 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line235>
> >
> >     should the pool be set to null after closing it? to prevent its being closed multiple times.

It is final, but I'll update closed as well. This all is only needed for tests.


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 278 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line278>
> >
> >     * Do you see `key` is used in other implementation? is it a necessary parameter in the signature?
> >     * can we rename `destroyObject` to `destory`, to on-par with `create()`?

That's the object pool API, can't change either of these


> On May 16, 2017, 9:07 p.m., Lei Xu wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717927#file1717927line79>
> >
> >     Should we also put the class name in the string?

Do you think it is useful? IMO it will only clatter output. I can add it if you think it is really useful.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175158
-----------------------------------------------------------


On May 13, 2017, 10:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 10:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Lei Xu <le...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175158
-----------------------------------------------------------




sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
Line 362 (original), 362 (patched)
<https://reviews.apache.org/r/59167/#comment248530>

    LOG.error(e) ?
    
    Btw, should it re-throw the exception?



sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
Line 395 (original), 393 (patched)
<https://reviews.apache.org/r/59167/#comment248531>

    LOG the Exception and/or rethrow it?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
Lines 374 (patched)
<https://reviews.apache.org/r/59167/#comment248532>

    Log.error(e) and/or re-throw it.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 150 (patched)
<https://reviews.apache.org/r/59167/#comment248536>

    What is the advantages of random pick against round-robin?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 173 (patched)
<https://reviews.apache.org/r/59167/#comment248545>

    If all addresses throw `NoSuchElementException`, will it throw a null in the end of the method?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 235 (patched)
<https://reviews.apache.org/r/59167/#comment248537>

    should the pool be set to null after closing it? to prevent its being closed multiple times.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 245 (patched)
<https://reviews.apache.org/r/59167/#comment248538>

    Could you add comments for `id`?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 278 (patched)
<https://reviews.apache.org/r/59167/#comment248543>

    * Do you see `key` is used in other implementation? is it a necessary parameter in the signature?
    * can we rename `destroyObject` to `destory`, to on-par with `create()`?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java
Lines 79 (patched)
<https://reviews.apache.org/r/59167/#comment248544>

    Should we also put the class name in the string?


- Lei Xu


On May 13, 2017, 3:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 3:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 22, 2017, 5:54 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
> > Lines 35 (patched)
> > <https://reviews.apache.org/r/59167/diff/5/?file=1722631#file1722631line35>
> >
> >     How are changes to UpdatableCache related to this patch?

They are needed for Kafka e2ee tests to pass


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175433
-----------------------------------------------------------


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175433
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java
Lines 35 (patched)
<https://reviews.apache.org/r/59167/#comment248877>

    How are changes to UpdatableCache related to this patch?


- kalyan kumar kalvagadda


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175425
-----------------------------------------------------------




sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java
Line 169 (original), 180 (patched)
<https://reviews.apache.org/r/59167/#comment248873>

    This string has the same name as the string defined in line 103, but has different value. 
    
    It is confusing, and easy for developer to make a mistake using wrong string. Can we have different name?


- Na Li


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 18, 2017, 8:18 p.m., Na Li wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
> > Line 113 (original), 112 (patched)
> > <https://reviews.apache.org/r/59167/diff/5/?file=1722613#file1722613line114>
> >
> >     Should you get client again when the current client is bad, so the following retry could succeed?

We don't need to get a client again - this will invalidate the client's transport which will cause a retry on the next attempt.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175427
-----------------------------------------------------------


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175427
-----------------------------------------------------------




sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java
Line 113 (original), 112 (patched)
<https://reviews.apache.org/r/59167/#comment248874>

    Should you get client again when the current client is bad, so the following retry could succeed?


- Na Li


On May 17, 2017, 4:42 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 4:42 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/5/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 26, 2017, 8:52 p.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Fixed kerberos issues discovered on real cluster deployment


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 74aced271dc28aeb294844ec7c0c2b78a0cdb9a3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-dist/pom.xml c002337153e9a4e781b0417cf4be6729bda84935 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java e23d13ba21b1042a7e73dd8807018fa9899be609 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 9beb07b43b405725f6f5e9b5d0d5c36f22f4cad8 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/7/

Changes: https://reviews.apache.org/r/59167/diff/6-7/


Testing
-------

I used the interactive shell to test failover across two different servers.


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 24, 2017, 12:10 p.m., kalyan kumar kalvagadda wrote:
> >

Seems like you have an emty comment.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175920
-----------------------------------------------------------


On May 24, 2017, 7:49 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 24, 2017, 7:49 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 74aced271dc28aeb294844ec7c0c2b78a0cdb9a3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java e23d13ba21b1042a7e73dd8807018fa9899be609 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 9beb07b43b405725f6f5e9b5d0d5c36f22f4cad8 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/6/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175920
-----------------------------------------------------------



- kalyan kumar kalvagadda


On May 24, 2017, 7:49 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 24, 2017, 7:49 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 74aced271dc28aeb294844ec7c0c2b78a0cdb9a3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java e23d13ba21b1042a7e73dd8807018fa9899be609 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 9beb07b43b405725f6f5e9b5d0d5c36f22f4cad8 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/6/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 24, 2017, 7:49 a.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Merged with latest state - especially with Kerberos fixes
Addressed code review comments.


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 74aced271dc28aeb294844ec7c0c2b78a0cdb9a3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java e23d13ba21b1042a7e73dd8807018fa9899be609 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 9beb07b43b405725f6f5e9b5d0d5c36f22f4cad8 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/6/

Changes: https://reviews.apache.org/r/59167/diff/5-6/


Testing
-------

I used the interactive shell to test failover across two different servers.


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 17, 2017, 4:42 a.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Addressed code review comments from Lei, Sergio and Kalyan


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/5/

Changes: https://reviews.apache.org/r/59167/diff/4-5/


Testing
-------

I used the interactive shell to test failover across two different servers.


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 135 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line135>
> >
> >     lifo is is set to ture by default. Should that be set explicitly? Can you explain what does it actuall do?

Doesn't hurt to be explicit. We want to use the most recent connection first.


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 148 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line148>
> >
> >     This logic creates a pool of connections. These connections could be spread accorss the servers that are up. Connections may or man not be evenly balanced accross the servers. Is this intent?

With pool it is rather difficult to achieve an even balancing. Do you think it is really important? We can try tp skew load balancing giving preference to a pool which has more idle connections, but this can disbalance load. I think we can fine-tune it once we get more practical experience.


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 161 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line161>
> >
> >     There is a confguration "sentry.service.client.connection.full.retry-total" that we had. This configuration is decides number of times to loop the complete list of servers when all the servers are down. This retry logic is removed now. You may want to add it.

I never understood why we have this setting in the first place - we do retry at the higher level by retrying client. Do you think it brings any value?


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 165 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line165>
> >
> >     when borrow object is called, it return an idle connection if it exists, if not it creates one. 
> >     
> >     If the server with address "addr" is down the underneath poolFactory throws TTransportException. What does pool return in that case?

It will throw an exception and we will either continue to the next one or throw an exception to the caller


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 201 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717926#file1717926line201>
> >
> >     now that you have seperate the client from the connection. It should be easy to retian the connection even when the client is closed and re-use it.
> >     
> >     How about creating a pool of size 1 when the pool is disabled?

min and max can be tuned if needed. The pool is disabled for HDFS clients where this isn't needed. Otherwise we should just use the pool.


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
> > Lines 63 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717933#file1717933line63>
> >
> >     As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?

I wanted to separate it for testing - there is a comment about it somewhere. This allows us to create test pool which doesn't connect to anything at all and creates fake objects. I have not done any specific tests yet - but that's the intention.


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717940#file1717940line72>
> >
> >     As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?

See the answer above.


> On May 17, 2017, 12:27 a.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
> > Lines 66 (patched)
> > <https://reviews.apache.org/r/59167/diff/4/?file=1717948#file1717948line67>
> >
> >     As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?

See the reply above.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175145
-----------------------------------------------------------


On May 13, 2017, 10:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 10:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review175145
-----------------------------------------------------------




sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 127 (patched)
<https://reviews.apache.org/r/59167/#comment248547>

    typo "Do not limit"



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 135 (patched)
<https://reviews.apache.org/r/59167/#comment248569>

    lifo is is set to ture by default. Should that be set explicitly? Can you explain what does it actuall do?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 148 (patched)
<https://reviews.apache.org/r/59167/#comment248566>

    This logic creates a pool of connections. These connections could be spread accorss the servers that are up. Connections may or man not be evenly balanced accross the servers. Is this intent?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 161 (patched)
<https://reviews.apache.org/r/59167/#comment248568>

    There is a confguration "sentry.service.client.connection.full.retry-total" that we had. This configuration is decides number of times to loop the complete list of servers when all the servers are down. This retry logic is removed now. You may want to add it.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 165 (patched)
<https://reviews.apache.org/r/59167/#comment248571>

    when borrow object is called, it return an idle connection if it exists, if not it creates one. 
    
    If the server with address "addr" is down the underneath poolFactory throws TTransportException. What does pool return in that case?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 201 (patched)
<https://reviews.apache.org/r/59167/#comment248567>

    now that you have seperate the client from the connection. It should be easy to retian the connection even when the client is closed and re-use it.
    
    How about creating a pool of size 1 when the pool is disabled?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 220 (patched)
<https://reviews.apache.org/r/59167/#comment248573>

    Here you are invalidating the complete pool. You are removed pooled instances with the specified key. You may want to update the comment.



sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java
Lines 63 (patched)
<https://reviews.apache.org/r/59167/#comment248534>

    As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java
Line 89 (original), 91 (patched)
<https://reviews.apache.org/r/59167/#comment248520>

    You have same function name getTransport to get TTransportWrapper from pool and TTransport from TTransportWrapper. You may want to rename them. It is confusing.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java
Lines 72 (patched)
<https://reviews.apache.org/r/59167/#comment248533>

    As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
Lines 66 (patched)
<https://reviews.apache.org/r/59167/#comment248535>

    As we do not have specific TransportFactory for specific clients, why should we expose it to client factory? Why not have it as a implementation detail of the SentryTransportPool?


- kalyan kumar kalvagadda


On May 13, 2017, 10:01 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 13, 2017, 10:01 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/4/
> 
> 
> Testing
> -------
> 
> I used the interactive shell to test failover across two different servers.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 13, 2017, 10:01 p.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Abstract out SentryTransportFactory


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java 651173ebb7df995ff0c432e2ebec08b4d95c3a77 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/4/

Changes: https://reviews.apache.org/r/59167/diff/3-4/


Testing
-------

I used the interactive shell to test failover across two different servers.


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 13, 2017, 9:43 p.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/3/


Testing (updated)
-------

I used the interactive shell to test failover across two different servers.


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 12, 2017, 7:06 a.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Addressed code review comments by Vamsee


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/3/

Changes: https://reviews.apache.org/r/59167/diff/2-3/


Testing
-------


Thanks,

Alexander Kolbasov


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
> > Line 362 (original), 362 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715177#file1715177line362>
> >
> >     I wonder if this goes into the process log or the stdout/stderr? But better to use LOGGER i guess.
> >     
> >     And in addition to that, this will swallow the exception and not raise it.

We are no longer using this class, this was needed for it to pass compilation - it will be removed soon.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
> > Lines 374 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715179#file1715179line374>
> >
> >     Same comment as above.

Same thing - this class is not used.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java
> > Line 99 (original), 110 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715186#file1715186line111>
> >
> >     Does this connection pooling create and keep the connections on demand basis? Or it tries to establish this number of connections before proceeding further?

It will try to set up min idle connections and keep number of idle connections between min and max, but will allow more active connections if needed. We may need to play with these a bit to tune to best values.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java
> > Lines 155 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715190#file1715190line322>
> >
> >     Even though the rest of this class is ThreadSafe, this invocation of USerGroupInformation is not. If there was a case where there are differences in the conf supplied, this could result in a strange behavior. But it is highly unlikely that conf/ changes within the running process.

The way we construct this it can't happen because transport factory has a fixed config for the pool. Some tests run with different configs and they create new pool with a new factory which has different config. For production run config never changes.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 88 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715191#file1715191line88>
> >
> >     Just wondering, is this thread safe?

It isn't, but this is used strictly for testing and debug logging - we have a single pool in production, so we don't care. Tests don't do this in parallel either. I'll add a comment that it is test-only.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715191#file1715191line94>
> >
> >     Shouldn't this come at the end of this constructor?

I removed this line altogether.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 160 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715191#file1715191line160>
> >
> >     Will commons-pool ensure that the object being returned has valid active connection?

It will not since we are not enabling pre-borrow check. Instead the caller will detect that the connection is broken and invalidate it.


> On May 12, 2017, 2:26 a.m., Vamsee Yarlagadda wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
> > Lines 250 (patched)
> > <https://reviews.apache.org/r/59167/diff/2/?file=1715191#file1715191line250>
> >
> >     nit. typo

fixed


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review174745
-----------------------------------------------------------


On May 11, 2017, 7:17 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 11, 2017, 7:17 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/#review174745
-----------------------------------------------------------




sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java
Line 362 (original), 362 (patched)
<https://reviews.apache.org/r/59167/#comment247960>

    I wonder if this goes into the process log or the stdout/stderr? But better to use LOGGER i guess.
    
    And in addition to that, this will swallow the exception and not raise it.



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
Lines 374 (patched)
<https://reviews.apache.org/r/59167/#comment247962>

    Same comment as above.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java
Line 99 (original), 110 (patched)
<https://reviews.apache.org/r/59167/#comment247977>

    Does this connection pooling create and keep the connections on demand basis? Or it tries to establish this number of connections before proceeding further?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java
Line 288 (original), 133 (patched)
<https://reviews.apache.org/r/59167/#comment247980>

    nit. Can we have this message before returning from this method that a secure connection was established?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java
Lines 155 (patched)
<https://reviews.apache.org/r/59167/#comment247979>

    Even though the rest of this class is ThreadSafe, this invocation of USerGroupInformation is not. If there was a case where there are differences in the conf supplied, this could result in a strange behavior. But it is highly unlikely that conf/ changes within the running process.



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 88 (patched)
<https://reviews.apache.org/r/59167/#comment247981>

    Just wondering, is this thread safe?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 94 (patched)
<https://reviews.apache.org/r/59167/#comment247982>

    Shouldn't this come at the end of this constructor?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 160 (patched)
<https://reviews.apache.org/r/59167/#comment247985>

    Will commons-pool ensure that the object being returned has valid active connection?



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 199-200 (patched)
<https://reviews.apache.org/r/59167/#comment247984>

    This message could be a bit confusing if isPoolEnabled is set to false



sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java
Lines 250 (patched)
<https://reviews.apache.org/r/59167/#comment247986>

    nit. typo



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
Line 30 (original), 31 (patched)
<https://reviews.apache.org/r/59167/#comment247987>

    ThreadSafe annotation?


- Vamsee Yarlagadda


On May 11, 2017, 7:17 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59167/
> -----------------------------------------------------------
> 
> (Updated May 11, 2017, 7:17 a.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1580
>     https://issues.apache.org/jira/browse/SENTRY-1580
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1580: Provide pooled client connection model with HA
> 
> 
> Diffs
> -----
> 
>   pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
>   sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
>   sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
>   sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
>   sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
>   sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
>   sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
>   sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
>   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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
>   sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
>   sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 
> 
> 
> Diff: https://reviews.apache.org/r/59167/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 59167: SENTRY-1580: Provide pooled client connection model with HA

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59167/
-----------------------------------------------------------

(Updated May 11, 2017, 7:17 a.m.)


Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na Li, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Fixed issues with testClientWithSmallMaxMsgSize


Bugs: SENTRY-1580
    https://issues.apache.org/jira/browse/SENTRY-1580


Repository: sentry


Description
-------

SENTRY-1580: Provide pooled client connection model with HA


Diffs (updated)
-----

  pom.xml ad54cfda5e7dabf9ecadc8f8d9f0e3012596fa32 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java a3140f2e23188e0bc7b6d5521a2f457d090a937f 
  sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 262db11677c1bb999265a5033971c06def9455ed 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java da587d00aa6e4b2fb6d78e3a720464d25bdcb47c 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java fdb6df44953b20dfe655c62d6cbd935b7ba15bf4 
  sentry-binding/sentry-binding-kafka/src/main/java/org/apache/sentry/kafka/binding/KafkaAuthBinding.java 48511145098215ce5d077a3d335ac659a2fbbf95 
  sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java 240067370b434054addfff00b985fb8c94b2bad9 
  sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java 84a61cc0bb5a7d5ba6ca3d74d7bcd3e66a8f351b 
  sentry-core/sentry-core-common/pom.xml e1be256e9a4b867215afaf94b93cae2cc5e417b1 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java 34a594eb24ab9de8ddfa91494e9c2c7d794e1129 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java 9ea7185c7a4660a4455a1f6436942043a7f81519 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java c0768f96d3e702b258db3c52a0cf4bff0f890e49 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java 2d8082787935bf3111f917761db3bc2d99f59df3 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java c97fe97f67ba6e3ee64785d8baed266db3f7daca 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryServiceClient.java 9a10ca5aea7fff8a97394bf325fcdfe4418f91a5 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java 9b9f9e84848dad02ab40dff2fa0232cecdebb878 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportPool.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TTransportWrapper.java PRE-CREATION 
  sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/ThriftUtil.java d9fab86727919496fd932407b3abf1a9d879f7e0 
  sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java 34caa0ea845fa3f57b16212367397b3525e5a785 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java 11f6894820f1df3102114787fa5fe2c060619541 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java 798bbef98b29af7c885c5ea747d77d2dad6c1693 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java e350103cfc31b4930e2a0cfdeeabc5d6e3615c4b 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83bdf65db618a912835c88c4051b9b41b4df 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java 75d2993a4a7a86b99f206a82b9fe751e79a04adb 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java 134012d18d75d12e3ccce82dac8145bfc41609f5 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java 41708c371b86e000e94ac78247bfb3bfc6c4a252 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java 11cdee7f1be13fae10028e7ab20e0a3b06e75237 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b7ac640ed4e616d886b801a0cddb4b7990e7601b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientFactory.java 46ac4a3348f50ab1c02e09f3e1eea59690392f4e 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryConfigToolSolr.java 404adb8fa612d5e1026a0cf96a65a98db4ca10c3 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellKafka.java d6d9014a90b9171d02363d0f9e651891ba8c531d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java 695c008c03c927b1126423bb61c52f5e3c5b18b9 
  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 d1a4d99f8080fc6178770986acdd101d6fe997f7 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryShellHive.java 1d0984671ab5014b0a0ace263b7f9967e013b7ee 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java acf9b05281d8456ebdf77414dc7886f9aa4ef28f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java 7db93101c7f7534ea2c6e67ae5d661cbca8411b1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java 0164fa6197822bf450754dc1fd116581c2321d0f 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a65634ce3fb07952782fb8fdc4d99e7f3f9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java a2027750add889e31ec1bb9b2a287d046e6c6343 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead00309f392b916c1ef5983248dedce3b99521 
  sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java 0b1ef68a55a1d40637f771759735fc0838525a40 
  sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c5c4ec58eded5621ad94ee28bc68d66c0b 


Diff: https://reviews.apache.org/r/59167/diff/2/

Changes: https://reviews.apache.org/r/59167/diff/1-2/


Testing
-------


Thanks,

Alexander Kolbasov