You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Anne Yu <an...@cloudera.com> on 2016/04/22 00:21:20 UTC
Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/
-----------------------------------------------------------
Review request for sentry, Hao Hao and Lenni Kuff.
Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583
Repository: sentry
Description
-------
Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8515a2ba1fc08104a788ea054f2b871905359273
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
Diff: https://reviews.apache.org/r/46542/diff/
Testing
-------
Tested on a unmanaged real cluster.
--------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
Thanks,
Anne Yu
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Anne Yu <an...@cloudera.com>.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, line 124
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line124>
> >
> > What are the HDFS specif tearDown here?Will the tearDown be the same when enableHDFSAcls is true?
Previously thought about cleaning up all files/dirs under prefix path but forgot to add. Will add them in the next patch. Thanks for pointing it out.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, line 173
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line173>
> >
> > Could you add the comments here to explain the retry logic for getting the acls? Also for the recursive verify logic?
It takes some time for acls to be synced up on hdfs files/dirs; Especially for these on external prefix path. On my testing cluster (2x large), takes >= 10s.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, line 204
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line204>
> >
> > Commnet to explain why we need this?
In case the roles haven't be cleaned up correctly from previous runs. Don't want tests to fail because of it. Since most of the time, roles don't exit, might throw exception, so just put all these logic in a convenience method.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, line 208
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line208>
> >
> > Log the exception here?
OK.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java, line 47
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line47>
> >
> > Do we need to make the max_num_of_groups configurable for different value testing? e.g make it > 32?
Yeah, for table dir, it will have max_num_of_groups*2 acl entries; for partition dir, it will have max_num_of_groups*3 acl entries. More acl entries put into the dir, longer time to sync up acls across components. So I chose 20.
> On April 25, 2016, 12:40 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java, line 68
> > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line68>
> >
> > How about test for external table outside of prefix? To check the ACLs are not changed by Sentry?
You meant negative test? Could add one for sure. Will do it in the next patch.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/#review130332
-----------------------------------------------------------
On April 21, 2016, 11:47 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:47 p.m.)
>
>
> Review request for sentry, Hao Hao and Lenni Kuff.
>
>
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8515a2ba1fc08104a788ea054f2b871905359273
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
>
> Diff: https://reviews.apache.org/r/46542/diff/
>
>
> Testing
> -------
>
> Tested on a unmanaged real cluster.
>
> --------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
>
> Results :
>
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/#review130332
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java (line 124)
<https://reviews.apache.org/r/46542/#comment194053>
What are the HDFS specif tearDown here?Will the tearDown be the same when enableHDFSAcls is true?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java (line 173)
<https://reviews.apache.org/r/46542/#comment194055>
Could you add the comments here to explain the retry logic for getting the acls? Also for the recursive verify logic?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java (line 204)
<https://reviews.apache.org/r/46542/#comment194058>
Commnet to explain why we need this?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java (line 208)
<https://reviews.apache.org/r/46542/#comment194057>
Log the exception here?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java (line 218)
<https://reviews.apache.org/r/46542/#comment194059>
Comments for this method.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java (line 47)
<https://reviews.apache.org/r/46542/#comment194061>
Do we need to make the max_num_of_groups configurable for different value testing? e.g make it > 32?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java (line 68)
<https://reviews.apache.org/r/46542/#comment194062>
How about test for external table outside of prefix? To check the ACLs are not changed by Sentry?
- Hao Hao
On April 21, 2016, 11:47 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:47 p.m.)
>
>
> Review request for sentry, Hao Hao and Lenni Kuff.
>
>
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8515a2ba1fc08104a788ea054f2b871905359273
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
>
> Diff: https://reviews.apache.org/r/46542/diff/
>
>
> Testing
> -------
>
> Tested on a unmanaged real cluster.
>
> --------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
>
> Results :
>
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Anne Yu <an...@cloudera.com>.
> On May 4, 2016, 12:46 a.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java, line 69
> > <https://reviews.apache.org/r/46542/diff/3/?file=1367636#file1367636line69>
> >
> > Should we have similar test for MAX_NUM_OF_GROUPS > 20?
Let me just change this value to 33. Thanks Haohao.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/#review131601
-----------------------------------------------------------
On April 29, 2016, 8:44 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 8:44 p.m.)
>
>
> Review request for sentry, Hao Hao and Lenni Kuff.
>
>
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d2a1d36ee0572d5d24c200f631607e4cfa8168e3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/PrivilegeResultSet.java 8818c4c2b35b5a9979b7ca11a8a8548121bc348a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
>
> Diff: https://reviews.apache.org/r/46542/diff/
>
>
> Testing
> -------
>
> Tested on a unmanaged real cluster.
>
> --------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
>
> Results :
>
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/#review131601
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java (line 69)
<https://reviews.apache.org/r/46542/#comment195613>
Should we have similar test for MAX_NUM_OF_GROUPS > 20?
- Hao Hao
On April 29, 2016, 8:44 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46542/
> -----------------------------------------------------------
>
> (Updated April 29, 2016, 8:44 p.m.)
>
>
> Review request for sentry, Hao Hao and Lenni Kuff.
>
>
> Bugs: SENTRY-583
> https://issues.apache.org/jira/browse/SENTRY-583
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add one boundary HDFS SyncUp test to run on a real cluster;
> Also add some basic APIs for createing more HDFS SyncUp tests.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d2a1d36ee0572d5d24c200f631607e4cfa8168e3
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/PrivilegeResultSet.java 8818c4c2b35b5a9979b7ca11a8a8548121bc348a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
>
> Diff: https://reviews.apache.org/r/46542/diff/
>
>
> Testing
> -------
>
> Tested on a unmanaged real cluster.
>
> --------------------------------------------------------
> T E S T S
> -------------------------------------------------------
> Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
>
> Results :
>
> Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/
-----------------------------------------------------------
(Updated April 29, 2016, 8:44 p.m.)
Review request for sentry, Hao Hao and Lenni Kuff.
Changes
-------
Addressed previous comments;
Added more validation to column level privileges and synced up acls;
Added negative test case;
Since they are boundary tests, splitted internal path tests and external path tests in different files; otherwise, test class runs long time.
Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583
Repository: sentry
Description
-------
Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsExtMaxGroups.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d2a1d36ee0572d5d24c200f631607e4cfa8168e3
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/PrivilegeResultSet.java 8818c4c2b35b5a9979b7ca11a8a8548121bc348a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
Diff: https://reviews.apache.org/r/46542/diff/
Testing
-------
Tested on a unmanaged real cluster.
--------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
Thanks,
Anne Yu
Re: Review Request 46542: SENTRY-583: Add boundary condition test
coverage to HDFS synchronization test suite around max #of groups
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46542/
-----------------------------------------------------------
(Updated April 21, 2016, 11:47 p.m.)
Review request for sentry, Hao Hao and Lenni Kuff.
Changes
-------
I also added one more test case to test one scenario when there is an external path configured in the prefix.
Bugs: SENTRY-583
https://issues.apache.org/jira/browse/SENTRY-583
Repository: sentry
Description
-------
Add one boundary HDFS SyncUp test to run on a real cluster;
Also add some basic APIs for createing more HDFS SyncUp tests.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 8515a2ba1fc08104a788ea054f2b871905359273
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java e1881b4bfaa5923fd0dab2096f670b22b944f116
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 77af4329642807283e6cd3d2e3be837c5d3e0dfb
Diff: https://reviews.apache.org/r/46542/diff/
Testing
-------
Tested on a unmanaged real cluster.
--------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
Thanks,
Anne Yu