You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Vamsee Yarlagadda <va...@cloudera.com> on 2016/12/07 02:39:14 UTC

Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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

Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.


Repository: sentry


Description
-------

This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.

More reference:
Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/


Diffs
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 

Diff: https://reviews.apache.org/r/54464/diff/


Testing
-------

Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.

```bash
vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
...
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore

Results :

Tests run: 40, Failures: 0, Errors: 0, Skipped: 2

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 38.016s
[INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
[INFO] Final Memory: 59M/660M
[INFO] ------------------------------------------------------------------------
```


Thanks,

Vamsee Yarlagadda


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

Posted by Vamsee Yarlagadda <va...@cloudera.com>.

> On Dec. 7, 2016, 9:15 p.m., kalyan kumar kalvagadda wrote:
> > It's better to add some unit test each function that changes.

Added a new test.


- Vamsee


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


On Dec. 7, 2016, 2:39 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 2:39 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 38.016s
> [INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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



It's better to add some unit test each function that changes.

- kalyan kumar kalvagadda


On Dec. 7, 2016, 2:39 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 2:39 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 38.016s
> [INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

Posted by Vamsee Yarlagadda <va...@cloudera.com>.

> On Dec. 7, 2016, 9:46 p.m., Alexander Kolbasov wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 1258
> > <https://reviews.apache.org/r/54464/diff/1/?file=1578441#file1578441line1258>
> >
> >     It is neat, but does it actually work?

Yes, i tested this and the existing tests that has multiple groups and roles passed as well. Anyway will put up a dedicated test to verify this.


- Vamsee


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


On Dec. 7, 2016, 2:39 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 2:39 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 38.016s
> [INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1258)
<https://reviews.apache.org/r/54464/#comment229171>

    It is neat, but does it actually work?


- Alexander Kolbasov


On Dec. 7, 2016, 2:39 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 2:39 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 40, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 38.016s
> [INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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



The fix is good, a few suggestions for the test.


sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java (line 384)
<https://reviews.apache.org/r/54464/#comment229368>

    Can you split this into a separate test for groups and users? This will simplify merge with CDH which has groups but not users.



sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java (line 393)
<https://reviews.apache.org/r/54464/#comment229370>

    It would be better to write a test in a way that would allow a set of roles as input rather then a few fixed values. Then we can construct a test e.g. with 0, 1 and N values.



sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java (line 405)
<https://reviews.apache.org/r/54464/#comment229366>

    use createRole() instead here and below.



sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java (line 419)
<https://reviews.apache.org/r/54464/#comment229369>

    I think a more accurate test verifies that each roleName is in tRoles and each item in tRoles is in the {roleName{1,2,3}}set.
    In practice your test does test for this condition, but it is not generalizable for a set of roles.


- Alexander Kolbasov


On Dec. 8, 2016, 7:47 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 7:47 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-07 23:46:42.147 java[5054:534219] Unable to load realm info from SCDynamicStore
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.538 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 42.003s
> [INFO] Finished at: Wed Dec 07 23:47:00 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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


Ship it!




Ship It!

- Alexander Kolbasov


On Dec. 9, 2016, 1:16 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54464/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2016, 1:16 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.
> 
> More reference:
> Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 
> 
> Diff: https://reviews.apache.org/r/54464/diff/
> 
> 
> Testing
> -------
> 
> Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
> ...
> -------------------------------------------------------
>  T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-07 23:46:42.147 java[5054:534219] Unable to load realm info from SCDynamicStore
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.538 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 42.003s
> [INFO] Finished at: Wed Dec 07 23:47:00 PST 2016
> [INFO] Final Memory: 59M/660M
> [INFO] ------------------------------------------------------------------------
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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

(Updated Dec. 9, 2016, 1:16 a.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.


Changes
-------

Addressed Sasha's comments.


Repository: sentry


Description
-------

This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.

More reference:
Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/


Diffs (updated)
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 

Diff: https://reviews.apache.org/r/54464/diff/


Testing
-------

Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.

```bash
vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
...
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
2016-12-07 23:46:42.147 java[5054:534219] Unable to load realm info from SCDynamicStore
Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.538 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore

Results :

Tests run: 41, Failures: 0, Errors: 0, Skipped: 2

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 42.003s
[INFO] Finished at: Wed Dec 07 23:47:00 PST 2016
[INFO] Final Memory: 59M/660M
[INFO] ------------------------------------------------------------------------
```


Thanks,

Vamsee Yarlagadda


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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

(Updated Dec. 8, 2016, 7:47 a.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.


Repository: sentry


Description
-------

This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.

More reference:
Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/


Diffs
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 

Diff: https://reviews.apache.org/r/54464/diff/


Testing (updated)
-------

Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.

```bash
vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
...
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
2016-12-07 23:46:42.147 java[5054:534219] Unable to load realm info from SCDynamicStore
Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.538 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore

Results :

Tests run: 41, Failures: 0, Errors: 0, Skipped: 2

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 42.003s
[INFO] Finished at: Wed Dec 07 23:47:00 PST 2016
[INFO] Final Memory: 59M/660M
[INFO] ------------------------------------------------------------------------
```


Thanks,

Vamsee Yarlagadda


Re: Review Request 54464: SENTRY-1557: getRolesForGroups(), getRolesForUsers() does too many trips to the the DB

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

(Updated Dec. 8, 2016, 7:44 a.m.)


Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar kalvagadda.


Changes
-------

Addressed Kalyan and Sasha's comments.


Repository: sentry


Description
-------

This change will ensure that multiple calls to the DB are replaced with a single call that looks for multiple matching values.

More reference:
Checkout "Collections" section under http://etutorials.org/Programming/Java+data+objects/Chapter+9.+The+JDO+Query+Language/9.6+The+Query+Filter/


Diffs (updated)
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f773a4443e81c5cde3aca0056a2e33d528bf4ec9 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 

Diff: https://reviews.apache.org/r/54464/diff/


Testing
-------

Verified the changes by running the unit tests of TestSentryStore and all 40 tests passed.

```bash
vamsee-MBP:sentry-service-server vamsee$ mvn clean test -Dtest=TestSentryStore
...
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
2016-12-06 18:30:07.974 java[5930:877254] Unable to load realm info from SCDynamicStore
Tests run: 40, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.984 sec - in org.apache.sentry.provider.db.service.persistent.TestSentryStore

Results :

Tests run: 40, Failures: 0, Errors: 0, Skipped: 2

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 38.016s
[INFO] Finished at: Tue Dec 06 18:30:24 PST 2016
[INFO] Final Memory: 59M/660M
[INFO] ------------------------------------------------------------------------
```


Thanks,

Vamsee Yarlagadda