You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li <li...@cloudera.com> on 2017/07/27 21:10:58 UTC
Review Request 61192: SENTRY-1855: PERM PATH transactions can fail to
commit to the sentry database under load
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61192/
-----------------------------------------------------------
Review request for sentry.
Repository: sentry
Description
-------
1. Make perm changeID to be auto-increased for performance improvement
2. revert change in sentry-1824 to synchronize the perm changes in SentryStore
3. Return continuous delta list starting from requested changeID from SentryStore even when the delta list has hole (for example, the list is 1,2,3,5,6, return 1,2,3). If there is a hole at the front of the list, return full snapshot.
Note: this only applies to perm changes. path changeID is manually increased, so has no hole.
The changeID sent to NN in the full snapshot is in the same transaction of getting the full snapshot.
Diffs
-----
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java f2368b7
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 58878e5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java e29e780
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo e8982f6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java 4fe62bf
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 670bc5e
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java eb39b0d
Diff: https://reviews.apache.org/r/61192/diff/1/
Testing
-------
Tested by Vamsee in his cluster
Thanks,
Na Li
Re: Review Request 61192: SENTRY-1855: PERM PATH transactions can
fail to commit to the sentry database under load
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
> On July 28, 2017, 3:44 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java
> > Line 70 (original), 70 (patched)
> > <https://reviews.apache.org/r/61192/diff/1/?file=1784753#file1784753line70>
> >
> > the changeID of MSentryPermChange is not assigned at input of constructor, so there is no way to set it in test. Therefore, its value is always 0. There is no benefit to have this test
We can have a test only visible constructor right that helps us in testing w/ MSentryPermChanges in general. The above test tries to validate some helper functions that run on top of MSentryPermChange and MSentryPathChange
- Vamsee
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61192/#review181643
-----------------------------------------------------------
On July 28, 2017, 7:26 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61192/
> -----------------------------------------------------------
>
> (Updated July 28, 2017, 7:26 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, kalyan kumar kalvagadda, Sergio Pena, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> 1. Make perm changeID to be auto-increased for performance improvement
> 2. revert change in sentry-1824 to synchronize the perm changes in SentryStore
> 3. Return continuous delta list starting from requested changeID from SentryStore even when the delta list has hole (for example, the list is 1,2,3,5,6, return 1,2,3). If there is a hole at the front of the list, return full snapshot.
> Note: this only applies to perm changes. path changeID is manually increased, so has no hole.
> The changeID sent to NN in the full snapshot is in the same transaction of getting the full snapshot.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java f2368b7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 58878e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java e29e780
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo e8982f6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java 4fe62bf
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 670bc5e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java eb39b0d
>
>
> Diff: https://reviews.apache.org/r/61192/diff/1/
>
>
> Testing
> -------
>
> Tested by Vamsee in his cluster
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 61192: SENTRY-1855: PERM PATH transactions can
fail to commit to the sentry database under load
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61192/#review181643
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java
Line 70 (original), 70 (patched)
<https://reviews.apache.org/r/61192/#comment257270>
the changeID of MSentryPermChange is not assigned at input of constructor, so there is no way to set it in test. Therefore, its value is always 0. There is no benefit to have this test
- Na Li
On July 27, 2017, 9:10 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61192/
> -----------------------------------------------------------
>
> (Updated July 27, 2017, 9:10 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> 1. Make perm changeID to be auto-increased for performance improvement
> 2. revert change in sentry-1824 to synchronize the perm changes in SentryStore
> 3. Return continuous delta list starting from requested changeID from SentryStore even when the delta list has hole (for example, the list is 1,2,3,5,6, return 1,2,3). If there is a hole at the front of the list, return full snapshot.
> Note: this only applies to perm changes. path changeID is manually increased, so has no hole.
> The changeID sent to NN in the full snapshot is in the same transaction of getting the full snapshot.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java f2368b7
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 58878e5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java e29e780
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo e8982f6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java 4fe62bf
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 670bc5e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/model/TestMSentryUtil.java eb39b0d
>
>
> Diff: https://reviews.apache.org/r/61192/diff/1/
>
>
> Testing
> -------
>
> Tested by Vamsee in his cluster
>
>
> Thanks,
>
> Na Li
>
>