----------------------------------------------------------- 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 > >
