Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[Na Li via Review Board]

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


Ship it!




Ship It!

- Na Li


On Nov. 17, 2017, 10:29 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63896/
> ---
> 
> (Updated Nov. 17, 2017, 10:29 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: sentry-2052
> https://issues.apache.org/jira/browse/sentry-2052
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch has a couple of improvements:
> * Set the transaction retry max to 1 instead of 10. This high number was 
> causing much of the overhead on the tests. I could not set to 0 because the 
> TestSentryStore takes more time for some reason.
> * Moved the SentryStore object creation from the @Before method to the 
> @BeforeClass method. This also save precious 35 seconds.
> 
> With the patch the time is reduced from 3min to 45 seconds.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  cf83e7796fd5bc22c76cba6096b0a821e82b87a7 
> 
> 
> Diff: https://reviews.apache.org/r/63896/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[Sergio Pena via Review Board]

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

(Updated Nov. 17, 2017, 10:29 p.m.)


Review request for sentry.


Bugs: sentry-2052
https://issues.apache.org/jira/browse/sentry-2052


Repository: sentry


Description
---

This patch has a couple of improvements:
* Set the transaction retry max to 1 instead of 10. This high number was 
causing much of the overhead on the tests. I could not set to 0 because the 
TestSentryStore takes more time for some reason.
* Moved the SentryStore object creation from the @Before method to the 
@BeforeClass method. This also save precious 35 seconds.

With the patch the time is reduced from 3min to 45 seconds.


Diffs (updated)
-

  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
 cf83e7796fd5bc22c76cba6096b0a821e82b87a7 


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

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


Testing
---


Thanks,

Sergio Pena

Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[Sergio Pena via Review Board]

> On Nov. 17, 2017, 5:31 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
> > Line 144 (original), 141 (patched)
> > 
> >
> > With the change same sentry store will be used for all the tests. 
> > Please document the change so tha people adding tests this class are aware 
> > of this.

Other tests do not document this, what's special to document this here? 
SentryStore is just an interface used to access the DB (devs should know this, 
don't they)?


- Sergio


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


On Nov. 17, 2017, 12:08 a.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63896/
> ---
> 
> (Updated Nov. 17, 2017, 12:08 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: sentry-2052
> https://issues.apache.org/jira/browse/sentry-2052
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch has a couple of improvements:
> * Set the transaction retry max to 1 instead of 10. This high number was 
> causing much of the overhead on the tests. I could not set to 0 because the 
> TestSentryStore takes more time for some reason.
> * Moved the SentryStore object creation from the @Before method to the 
> @BeforeClass method. This also save precious 35 seconds.
> 
> With the patch the time is reduced from 3min to 45 seconds.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  cf83e7796fd5bc22c76cba6096b0a821e82b87a7 
> 
> 
> Diff: https://reviews.apache.org/r/63896/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[Sergio Pena via Review Board]

> On Nov. 17, 2017, 4:45 p.m., Na Li wrote:
> > Have you run the tests for several times to make sure each test cleans up 
> > the sentry store, so next test won't be affected by previous test state?

Yes, I have done that and it works.


- Sergio


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


On Nov. 17, 2017, 12:08 a.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63896/
> ---
> 
> (Updated Nov. 17, 2017, 12:08 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: sentry-2052
> https://issues.apache.org/jira/browse/sentry-2052
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch has a couple of improvements:
> * Set the transaction retry max to 1 instead of 10. This high number was 
> causing much of the overhead on the tests. I could not set to 0 because the 
> TestSentryStore takes more time for some reason.
> * Moved the SentryStore object creation from the @Before method to the 
> @BeforeClass method. This also save precious 35 seconds.
> 
> With the patch the time is reduced from 3min to 45 seconds.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  cf83e7796fd5bc22c76cba6096b0a821e82b87a7 
> 
> 
> Diff: https://reviews.apache.org/r/63896/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[kalyan kumar kalvagadda via Review Board]

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




sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
Line 144 (original), 141 (patched)


With the change same sentry store will be used for all the tests. Please 
document the change so tha people adding tests this class are aware of this.


- kalyan kumar kalvagadda


On Nov. 17, 2017, 12:08 a.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63896/
> ---
> 
> (Updated Nov. 17, 2017, 12:08 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: sentry-2052
> https://issues.apache.org/jira/browse/sentry-2052
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch has a couple of improvements:
> * Set the transaction retry max to 1 instead of 10. This high number was 
> causing much of the overhead on the tests. I could not set to 0 because the 
> TestSentryStore takes more time for some reason.
> * Moved the SentryStore object creation from the @Before method to the 
> @BeforeClass method. This also save precious 35 seconds.
> 
> With the patch the time is reduced from 3min to 45 seconds.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  cf83e7796fd5bc22c76cba6096b0a821e82b87a7 
> 
> 
> Diff: https://reviews.apache.org/r/63896/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 63896: SENTRY-2052: Reduce TestSentryStore time by setting transaction retries to 1 and other refactors

[Na Li via Review Board]

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



Have you run the tests for several times to make sure each test cleans up the 
sentry store, so next test won't be affected by previous test state?

- Na Li


On Nov. 17, 2017, 12:08 a.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63896/
> ---
> 
> (Updated Nov. 17, 2017, 12:08 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Bugs: sentry-2052
> https://issues.apache.org/jira/browse/sentry-2052
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> This patch has a couple of improvements:
> * Set the transaction retry max to 1 instead of 10. This high number was 
> causing much of the overhead on the tests. I could not set to 0 because the 
> TestSentryStore takes more time for some reason.
> * Moved the SentryStore object creation from the @Before method to the 
> @BeforeClass method. This also save precious 35 seconds.
> 
> With the patch the time is reduced from 3min to 45 seconds.
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  cf83e7796fd5bc22c76cba6096b0a821e82b87a7 
> 
> 
> Diff: https://reviews.apache.org/r/63896/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>