> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/pom.xml, line 36
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540947#file1540947line36>
> >
> >     Why do we need Hive lib ?

Will remove this property, forgot to delete it.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/pom.xml, line 503
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540947#file1540947line503>
> >
> >     May be add some details here in a comment on what this profile is for 
> > and reason behind configuring it the way it is?

Will add come comments.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java,
> >  line 24
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540948#file1540948line24>
> >
> >     Not entirely sure why this commit requires changes in 
> > UnmanagedHiveServer? Can we add a comment to the commit message or RB 
> > summary?

The main reason is for jar file to read system property from outside. Will add 
comments into RB to explain.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 52
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line52>
> >
> >     Why copy hive-site?

Can remove this copy, but main purpose is to make sure hive-site.xml is in the 
HADOOP_CLASSPATH.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 60
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line60>
> >
> >     Is the name accurate?

Or can use "TestDataStats" instead. It stores the most recent statistics of 
test data: HMS and Sentry data.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 80
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line80>
> >
> >     Where are these defined?

NUM_OF_ROLES is defined in the test configuration file. Let me remove this 
extra variable then.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 93
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line93>
> >
> >     Does it make sense to also track cardinality of joins? Role - group, 
> > role - privilege? As most of our for loops and joins will be affected with 
> > these numbers.

The current implementation also maps roles to groups randomly; and privileges 
are granted to a random role also. You meant to print out their mapping? In the 
following patch for validation part of any non-functional tests, will have that 
feature to be able to layout the mapping between roles, groups and privileges.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  line 99
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line99>
> >
> >     Is it better to take a command line argument instead of environmnet 
> > variable?

Either one works here. Will modify it to a command line argument.


> On Oct. 18, 2016, 11:42 p.m., Sravya Tirukkovalur wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java,
> >  lines 130-131
> > <https://reviews.apache.org/r/52675/diff/4/?file=1540949#file1540949line130>
> >
> >     What these values correspond to?

The total number of roles and groups will be created from tool.


On Oct. 18, 2016, 11:42 p.m., Anne Yu wrote:
> > Can we add some description to the class itself on what inputs it expects 
> > and how it gets the values and privileges a user needs to run this tool?

In the next patch I will sumbit a longevity test tool, in which majority part 
is to validate existing metadata; I will provide a way for user to get sentry 
metadata and validate them.


- Anne


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


On Oct. 18, 2016, 10:19 p.m., Anne Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52675/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2016, 10:19 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, Li Li, and Sravya 
> Tirukkovalur.
> 
> 
> Bugs: SENTRY-1497
>     https://issues.apache.org/jira/browse/SENTRY-1497
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Specify the scale numbers like databases, tables, views, partitions, columns, 
> uris, privileges, role, and groups in a config file, the tool can create such 
> volume of data in Sentry and HMS databases. To speed up test, it can also do 
> the task parallelly.
> 
> 
> Diffs
> -----
> 
>   sentry-tests/sentry-tests-hive/pom.xml 
> a2512ee3919a3958425f4ab74b178d02e0402315 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/UnmanagedHiveServer.java
>  90713b1aaa688808859e670c8799f8e5be2d6d26 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/CreateSentryTestScaleData.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/TestTools.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/tools/sentry_scale_test_config.xml
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/scripts/scale-test/create-many-dbs-tables.sh
>  dcdddeb95a896ca8470d0b994f5460531e34d113 
> 
> Diff: https://reviews.apache.org/r/52675/diff/
> 
> 
> Testing
> -------
> 
> Most recent run uses scale configuration. Totaly running time is 725 secs 
> with 50 threads on a real cluster. 
> 
> Objects status: total databases(300); tables(1224), views(503), 
> partitions(5505), columns(1905);
> Privileges status: database privileges(299), table privileges(824), view 
> privileges(503), partition privileges(204), column privileges(299), uri 
> privileges(204); roles(1000), groups(500);
> failed threads(1), running time(725 secs)
> 
> 
> Thanks,
> 
> Anne Yu
> 
>

Reply via email to