You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Li Li <li...@cloudera.com> on 2015/12/03 08:10:14 UTC
Re: Review Request 40408: Add concurrent jdbc and thrift client tests
to ensure synchronization works fine
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40408/#review108787
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java (line 141)
<https://reviews.apache.org/r/40408/#comment168268>
should we move stmt.close() and connection.close() inside of finally?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java (line 163)
<https://reviews.apache.org/r/40408/#comment168267>
remove extral indention in line 163 and 166?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java (line 191)
<https://reviews.apache.org/r/40408/#comment168269>
How about use AtomicInteger which has better performance than synchrozied?
- Li Li
On Nov. 17, 2015, 11:06 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40408/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 11:06 p.m.)
>
>
> Review request for sentry, Hao Hao, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-906
> https://issues.apache.org/jira/browse/SENTRY-906
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add concurrent jdbc and thrift client tests to ensure synchronization works fine;
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
>
> Diff: https://reviews.apache.org/r/40408/diff/
>
>
> Testing
> -------
>
> I can successfully run locally this test.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 40408: Add concurrent jdbc and thrift client tests
to ensure synchronization works fine
Posted by Li Li <li...@cloudera.com>.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java, line 141
> > <https://reviews.apache.org/r/40408/diff/1/?file=1128920#file1128920line141>
> >
> > should we move stmt.close() and connection.close() inside of finally?
>
> Anne Yu wrote:
> Will move.
Thanks, Anne! Besides, should we finally close stmt and connection in other methods as well, eg. adminCleanUp, adminShowRole, adminGrant, createPartition, createDbTb ?
- Li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40408/#review108787
-----------------------------------------------------------
On Nov. 17, 2015, 11:06 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40408/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 11:06 p.m.)
>
>
> Review request for sentry, Hao Hao, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-906
> https://issues.apache.org/jira/browse/SENTRY-906
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add concurrent jdbc and thrift client tests to ensure synchronization works fine;
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
>
> Diff: https://reviews.apache.org/r/40408/diff/
>
>
> Testing
> -------
>
> I can successfully run locally this test.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 40408: Add concurrent jdbc and thrift client tests
to ensure synchronization works fine
Posted by Anne Yu <an...@cloudera.com>.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java, line 141
> > <https://reviews.apache.org/r/40408/diff/1/?file=1128920#file1128920line141>
> >
> > should we move stmt.close() and connection.close() inside of finally?
>
> Anne Yu wrote:
> Will move.
>
> Li Li wrote:
> Thanks, Anne! Besides, should we finally close stmt and connection in other methods as well, eg. adminCleanUp, adminShowRole, adminGrant, createPartition, createDbTb ?
Yeah, have noticed that also. Let me see if possible could do it.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40408/#review108787
-----------------------------------------------------------
On Nov. 17, 2015, 11:06 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40408/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 11:06 p.m.)
>
>
> Review request for sentry, Hao Hao, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-906
> https://issues.apache.org/jira/browse/SENTRY-906
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add concurrent jdbc and thrift client tests to ensure synchronization works fine;
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
>
> Diff: https://reviews.apache.org/r/40408/diff/
>
>
> Testing
> -------
>
> I can successfully run locally this test.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 40408: Add concurrent jdbc and thrift client tests
to ensure synchronization works fine
Posted by Anne Yu <an...@cloudera.com>.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> >
Thanks for your review.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java, line 191
> > <https://reviews.apache.org/r/40408/diff/1/?file=1128920#file1128920line191>
> >
> > How about use AtomicInteger which has better performance than synchrozied?
>From previous comments, seems using synchronized on a small api/class is cleaner than guard one variable. Maybe can keep it as it is.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java, line 141
> > <https://reviews.apache.org/r/40408/diff/1/?file=1128920#file1128920line141>
> >
> > should we move stmt.close() and connection.close() inside of finally?
Will move.
> On Dec. 3, 2015, 7:10 a.m., Li Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java, line 163
> > <https://reviews.apache.org/r/40408/diff/1/?file=1128920#file1128920line163>
> >
> > remove extral indention in line 163 and 166?
Will remove the whitespaces.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40408/#review108787
-----------------------------------------------------------
On Nov. 17, 2015, 11:06 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40408/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2015, 11:06 p.m.)
>
>
> Review request for sentry, Hao Hao, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-906
> https://issues.apache.org/jira/browse/SENTRY-906
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add concurrent jdbc and thrift client tests to ensure synchronization works fine;
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestConcurrentClients.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
>
> Diff: https://reviews.apache.org/r/40408/diff/
>
>
> Testing
> -------
>
> I can successfully run locally this test.
>
>
> Thanks,
>
> Anne Yu
>
>