You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arun Suresh <ar...@gmail.com> on 2014/12/10 09:01:25 UTC

Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

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

Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

The basic motivations for this JIRA is

1) Make the Sentry Store pluggable
2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
3) Separate out the core DataStructures and Persistence logic

Current status :
1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
3) The tests in sentry-hive-tests can be run against the new store impementationss :
   * add env variable USE_IN_MEM=true to use the in memory store
   * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
4) All TestSentrySeviceIntegration tests pass
5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore

TODO:
1) Add support for log compaction


Diffs
-----

  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 

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


Testing
-------


Thanks,

Arun Suresh


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28893/#review64702
-----------------------------------------------------------



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107503>

    Aah yes.. missed that.. thanks for pointing that out


- Arun Suresh


On Dec. 10, 2014, 8:01 a.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 8:01 a.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28893/#review64844
-----------------------------------------------------------



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107635>

    It's a very nice idea for using binary number to stand for privilege. I have a minor suggestion: if we add some privilege types in future, the ALL will also need to change, could we use an immutable number to stand for ALL? but this way may be not following the principle of least privilege.
    So just point out it, and feel free to fix or drop.


- Dapeng Sun


On 十二月 10, 2014, 4:01 p.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated 十二月 10, 2014, 4:01 p.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Dapeng Sun <da...@intel.com>.

> On 十二月 11, 2014, 3:12 p.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java, line 43
> > <https://reviews.apache.org/r/28893/diff/1/?file=788014#file788014line43>
> >
> >     If Sentry is running with HA, there will be a problem to synchronize the local files.
> 
> Arun Suresh wrote:
>     Agreed Colin.. I am working on an solution for HA (active / standby) mode.. Should be there in my next patch

Agreed both of you. Current HA solution should be deploy on a HA database, I think we may can stop to start service if both In_MEM and HA are enable. and it's great to implement a HA solution for InMEM_SentryStore.


- Dapeng


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


On 十二月 10, 2014, 4:01 p.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated 十二月 10, 2014, 4:01 p.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Arun Suresh <ar...@gmail.com>.

> On Dec. 11, 2014, 7:12 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java, line 43
> > <https://reviews.apache.org/r/28893/diff/1/?file=788014#file788014line43>
> >
> >     If Sentry is running with HA, there will be a problem to synchronize the local files.

Agreed Colin.. I am working on an solution for HA (active / standby) mode.. Should be there in my next patch


> On Dec. 11, 2014, 7:12 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java, line 123
> > <https://reviews.apache.org/r/28893/diff/1/?file=788014#file788014line123>
> >
> >     Maybe this exception need to be thrown, or sentry service will get the incorrect authorization data.

So this is an ObjectInputStream .. unlike other InputStreams where the read() method which returns a -1, this is the only way to signal an EOF.. it is not really an exception or error in the stream..


> On Dec. 11, 2014, 7:12 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java, line 299
> > <https://reviews.apache.org/r/28893/diff/1/?file=788015#file788015line299>
> >
> >     For the name check, maybe we can add test case to test case insentitive.

Yup.. a more comprehensive test suite to follow.. thanx for pointing it out..


> On Dec. 11, 2014, 7:12 a.m., Colin Ma wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java, line 97
> > <https://reviews.apache.org/r/28893/diff/1/?file=788025#file788025line97>
> >
> >     Maybe add another role named "NEWROLE" to test the case insensitive.

see above..


- Arun


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


On Dec. 10, 2014, 8:01 a.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 8:01 a.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28893/#review64698
-----------------------------------------------------------



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107501>

    If Sentry is running with HA, there will be a problem to synchronize the local files.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107498>

    Maybe this exception need to be thrown, or sentry service will get the incorrect authorization data.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107497>

    1. There should be a loop to read all reverted records?
    
    2. If there has an exception during revertOis.readInt(), ignore the exception maybe cause the data not correct.
    The reverted records won't be filtered. This exception should be thrown.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107496>

    How about using a map to get the Privilege:
    
    static Map<String, Privilege> actionPrivilegeMap = ........
    {
    actionPrivilegeMap.put(AccessConstants.ACTION_ALL, ALL);
    actionPrivilegeMap.put(AccessConstants.CREATE, CREATE);
    .........
    }
    
    static Privilege fromTSentryPrivilege(TSentryPrivilege priv) {
        Privilege result = actionPrivilegeMap.get(Strings.nullToEmpty(priv.getAction()));
        if (result == null) {
           result = NONE;
        }
        return result;
    }



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107500>

    For the name check, maybe we can add test case to test case insentitive.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107499>

    Maybe add another role named "NEWROLE" to test the case insensitive.


- Colin Ma


On Dec. 10, 2014, 8:01 a.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 8:01 a.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28893/#review64883
-----------------------------------------------------------


Some comments from an initial pass over. Overall it looks really cool. It would be good clarified the object model and persistence behavior someplace in the code.


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java
<https://reviews.apache.org/r/28893/#comment107755>

    Fix?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107689>

    Add class comment that explains the persistence behavior. Also the current class name sounds kind of like this is a SentryStore that logs more debug information, consider renaming.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107772>

    The current implementation seems to treat these logs as "unbounded". You will need to always store the complete history of all commits and then replay that when you start back up. This is probably okay for now, but it doesn't scale well. Add TODO or comment. 
    
    Also, it would be nice to have metrics on things like the time it took to recover, etc. Add them or add a TODO.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107771>

    Why is this Atomic?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107693>

    Preconditions.checkState(lastSeqNum < seqId). 
    
    nit: consitent use of seqNum or seqId but not both.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107741>

    This is just the initial prototype, but it's probably good to explictly call out the log format. We also might want to encode some version information in it early on to handle the case where the format changes.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107742>

    How will we detect the scenario where the log is corrupted?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java
<https://reviews.apache.org/r/28893/#comment107699>

    You are committing the record when it does not have an associated item in the set of reverted commits. What if your app started an operation, but crashed before writing out the revereted commits? Seems a bit dangerous, no?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107744>

    nit: define in hex to make this more clear. Also, does ALL belong in this enum? It seems like it should be defined dynamically by OR'ing all the privileges.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107746>

    clarify why



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107770>

    remove



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107748>

    consider a more descriptive name. What is this checking?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107773>

    getLeafAuthorizeable?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107774>

    what does isParentOk mean?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107775>

    The code in the if and else blocks looks nearly identical, consolidate?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107777>

    We should think about how we can start adding some abstraction on top of this so we don't have the db-model specific code wrapped in here. Not needed in this patch, but something to consider.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java
<https://reviews.apache.org/r/28893/#comment107778>

    There are lots of maps of maps around. Do you think it makes sense to create a data structure that hides these details?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java
<https://reviews.apache.org/r/28893/#comment107781>

    Who uses this? Is it used by the db-backed store? The FileLogger store? Would be good to make this clear.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java
<https://reviews.apache.org/r/28893/#comment107752>

    It would be nice if there was a bit more granular locking. For the time being should we just make everything synchronized?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java
<https://reviews.apache.org/r/28893/#comment107753>

    ?



sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
<https://reviews.apache.org/r/28893/#comment107754>

    "a Privilege in transport". Can you clarify? Also, if this is a privilege (singular) it's strange we include a set of privileges here.


- Lenni Kuff


On Dec. 10, 2014, 8:01 a.m., Arun Suresh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28893/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2014, 8:01 a.m.)
> 
> 
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The basic motivations for this JIRA is
> 
> 1) Make the Sentry Store pluggable
> 2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
> 3) Separate out the core DataStructures and Persistence logic
> 
> Current status :
> 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
> 2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
> 3) The tests in sentry-hive-tests can be run against the new store impementationss :
>    * add env variable USE_IN_MEM=true to use the in memory store
>    * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
> 4) All TestSentrySeviceIntegration tests pass
> 5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore
> 
> TODO:
> 1) Add support for log compaction
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
>   sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
>   sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 
> 
> Diff: https://reviews.apache.org/r/28893/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arun Suresh
> 
>


Re: Review Request 28893: SENTRY-567 : Add support for an alternate non-DB base SentryStore

Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28893/
-----------------------------------------------------------

(Updated Dec. 10, 2014, 8:01 a.m.)


Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.


Repository: sentry


Description
-------

The basic motivations for this JIRA is

1) Make the Sentry Store pluggable
2) Provide an implementation of SentryStore that in not dependent on JDO / Datanucleus or any external data layer.
3) Separate out the core DataStructures and Persistence logic

Current status :
1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore
2) It also includes a persistent FileLoggingSentryStore : This writes out sentry store operations to a log file and on restart, reads the log file and populates a backing InMemSentryStore
3) The tests in sentry-hive-tests can be run against the new store impementationss :
   * add env variable USE_IN_MEM=true to use the in memory store
   * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore with a backing in memory store
4) All TestSentrySeviceIntegration tests pass
5) There is a senparate TestInMemSentryStore that runs most of the same tests as TestSentryStore

TODO:
1) Add support for log compaction


Diffs
-----

  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java f1e792d 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java 1c68a0f 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f98e853 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java 55bec0b 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 29e3131 
  sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift 993ea46 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 8fbe3f4 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java 922cbc2 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java 0add58b 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 4a475ba 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java 2cecdfd 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java acb789f 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java f8cc1d0 

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


Testing
-------


Thanks,

Arun Suresh