> On June 27, 2017, 3:51 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 2531 (original), 2536 (patched)
> > <https://reviews.apache.org/r/59720/diff/9/?file=1762461#file1762461line2536>
> >
> >     The map is still from hiveObj to path so the comment isn't quite 
> > correct now.

Changed to this:
'The snapshot is represented by a snapshot ID, and a map from hiveObj to paths.'


> On June 27, 2017, 3:51 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 2578 (original), 2601 (patched)
> > <https://reviews.apache.org/r/59720/diff/9/?file=1762461#file1762461line2601>
> >
> >     Remove this part and just persist the mapping. This is 
> > performance-critical part of the code.

Then it does not make sense to have this private class with one line of code to 
persist the path. I will remove the method, and move the pm.makePersistent() to 
the caller instead.


- Sergio


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


On June 25, 2017, 11:02 a.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59720/
> -----------------------------------------------------------
> 
> (Updated June 25, 2017, 11:02 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and 
> Vamsee Yarlagadda.
> 
> 
> Bugs: SENTRY-1781
>     https://issues.apache.org/jira/browse/SENTRY-1781
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch only adds an extra column to the MAuthzPathsMapping table that 
> contains an image identifier that will be used to detect full snapshots. This 
> patch does not increment the identifier yet, it only uses 0 for now.
> 
> TIPS for reviewers:
> 
> - package.jdo adds a new column name AUTHZ_IMG_ID to the AUTHZ_MAPS_MAPPING 
> table used to identify the paths that are part of a new hive snapshot.
> 
> - MAuthzPathsMapping.java adds the the new column and API to handle JDO 
> requests.
> 
> - SentryStore.java modifies all methods that interact with the snapshots. It 
> makes sure that
>   when persisting a new snapshot, then the authzImgID is incremented and the 
> old one is
>   deleted. Also it makes sure that retrieve/add/delete/rename/update uses the 
> current 
>   authzImgID whenver they're called.
> 
> - TestSentryStore.java adds unit tests to verify that all SentryStore API 
> work correctly
>   when a new hive snapshot is created.
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  27999cfcdc09ded657b98d89b52360e42ed0071e 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  29878c5f1213062cb9d0a1f9666a19004b67dc52 
> 
> 
> Diff: https://reviews.apache.org/r/59720/diff/9/
> 
> 
> Testing
> -------
> 
> All unit tests run locally, but there are some flaky tests on Jenkins no 
> related to this patch.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Reply via email to