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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 684-687 (patched)
<https://reviews.apache.org/r/66792/#comment283732>

    I have two comments here
    1. This delete should be straggered. There could be huge data speread 
accross multiple snapshots. This is take much longer. If there are too manu 
snpshots we should limit on how many shots can be purged at one run. Let's say 
there are 20 snapshots already existing, we could have a behaviour where each 
time the purge is scheduled it cleans-up 5 snaphots. 
    2. we should find a way to delete entires in AUTHZ_PATH and 
AUTHZ_PATHS_MAPPING togerther in single datanuclesu call. that will be 
efficient.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 690-692 (patched)
<https://reviews.apache.org/r/66792/#comment283727>

    This is not efficient as you are stoing all the paths objects in memory 
before trying to delete them from the database. There could be millions of path 
entires.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 705 (patched)
<https://reviews.apache.org/r/66792/#comment283729>

    Provoding a huge list of patyh entires might be inefficient as datanucleus 
is going to perform a delete from each entiry



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 382-383 (patched)
<https://reviews.apache.org/r/66792/#comment283726>

    Isn't it better to have both of these in single transaction?


- kalyan kumar kalvagadda


On April 27, 2018, 3:57 a.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66792/
> -----------------------------------------------------------
> 
> (Updated April 27, 2018, 3:57 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Add function to periodically remove the old path snapshot
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  8ac3c0d 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  4236c02 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  a66d91e 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  b410027 
> 
> 
> Diff: https://reviews.apache.org/r/66792/diff/2/
> 
> 
> Testing
> -------
> 
> unit tests in TestSentryStore.
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to