You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org> on 2018/02/06 19:06:19 UTC
Review Request 65533: SENTRY-2115: Incorrect behavior of HMsFollower
when HDFSSync feature is disabled.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/
-----------------------------------------------------------
Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
Bugs: SENTRY-2115
https://issues.apache.org/jira/browse/SENTRY-2115
Repository: sentry
Description
-------
**Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
**Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
**Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
**New Behavior:** Full snapshots need not be taken in all
When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
**Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
**New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
Diff: https://reviews.apache.org/r/65533/diff/1/
Testing
-------
Made sure that all the tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 5:02 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line125>
> >
> > Should you update the following condition for DBUpdateForwarder.getAllUpdatesFrom()?
> >
> > Replace
> > if (seqNum > curSeqNum) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > }
> >
> > With
> > if (seqNum > curSeqNum) {
> > if(seqNum = curSeqNum + 1) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > } else {
> > // perm change has been reset, get full perm snapshot
> > List<K>fullImage = retrieveFullImage();
> > //Only log if we have received full image
> > if( !fullImage.isEmpty()) {
> > LOGGER.info("({}) A newer full update with image number {} was found and being sent to HDFS",
> > retrieverType, curImgNum);
> > }
> > return fullImage;
> > }
> > }
> >
> > Once you clear the perm change, the seqNum from Name Node will be larger than curSeqNum. The correct behavior should be to send full snapshot of permission to Name Node after you clear the perm changes.
>
> Na Li wrote:
> Another scenario to take care is that: 1) Name Node has synced with full snapshot before HDFS sync is disabled and is asking for seqNum = 1 after HDFS sync is enabled. A full snapshot should be sent to Name Node, but based on above logic, when there is no perm changes, empty list is sent back.
>
> kalyan kumar kalvagadda wrote:
> Enabling/disabling HDFS sync is done by changing an bunch of properties both in sentry server and namnode. When even it happens both sentry and namenode are restarted.
>
> When ever namenode starts, it starts from fresh and the seqNum will be 0.
>
> I'm dropping your comment. If you don't agree, plese re-open it with your comment.
>
> Na Li wrote:
> Sentry service has no control on Name Node service. When sentry configuration changes, it is not guaranteed that Name Node service will be restarted. I am OK if you
> 1) add another jira to properly address the issue I mentioned above (optional)
> 2) add comment in code for this assumption
> 3) add the document in sentry for this assumption
> and finish this jira.
>
> It is a dangerous design to assume a behavior that is not guaranteed in order for the feature to work properly.
This will not be an issue with the new patch
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review196971
-----------------------------------------------------------
On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 9 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 5:02 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line125>
> >
> > Should you update the following condition for DBUpdateForwarder.getAllUpdatesFrom()?
> >
> > Replace
> > if (seqNum > curSeqNum) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > }
> >
> > With
> > if (seqNum > curSeqNum) {
> > if(seqNum = curSeqNum + 1) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > } else {
> > // perm change has been reset, get full perm snapshot
> > List<K>fullImage = retrieveFullImage();
> > //Only log if we have received full image
> > if( !fullImage.isEmpty()) {
> > LOGGER.info("({}) A newer full update with image number {} was found and being sent to HDFS",
> > retrieverType, curImgNum);
> > }
> > return fullImage;
> > }
> > }
> >
> > Once you clear the perm change, the seqNum from Name Node will be larger than curSeqNum. The correct behavior should be to send full snapshot of permission to Name Node after you clear the perm changes.
>
> Na Li wrote:
> Another scenario to take care is that: 1) Name Node has synced with full snapshot before HDFS sync is disabled and is asking for seqNum = 1 after HDFS sync is enabled. A full snapshot should be sent to Name Node, but based on above logic, when there is no perm changes, empty list is sent back.
>
> kalyan kumar kalvagadda wrote:
> Enabling/disabling HDFS sync is done by changing an bunch of properties both in sentry server and namnode. When even it happens both sentry and namenode are restarted.
>
> When ever namenode starts, it starts from fresh and the seqNum will be 0.
>
> I'm dropping your comment. If you don't agree, plese re-open it with your comment.
Sentry service has no control on Name Node service. When sentry configuration changes, it is not guaranteed that Name Node service will be restarted. I am OK if you
1) add another jira to properly address the issue I mentioned above (optional)
2) add comment in code for this assumption
3) add the document in sentry for this assumption
and finish this jira.
It is a dangerous design to assume a behavior that is not guaranteed in order for the feature to work properly.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review196971
-----------------------------------------------------------
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 5:02 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line125>
> >
> > Should you update the following condition for DBUpdateForwarder.getAllUpdatesFrom()?
> >
> > Replace
> > if (seqNum > curSeqNum) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > }
> >
> > With
> > if (seqNum > curSeqNum) {
> > if(seqNum = curSeqNum + 1) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > } else {
> > // perm change has been reset, get full perm snapshot
> > List<K>fullImage = retrieveFullImage();
> > //Only log if we have received full image
> > if( !fullImage.isEmpty()) {
> > LOGGER.info("({}) A newer full update with image number {} was found and being sent to HDFS",
> > retrieverType, curImgNum);
> > }
> > return fullImage;
> > }
> > }
> >
> > Once you clear the perm change, the seqNum from Name Node will be larger than curSeqNum. The correct behavior should be to send full snapshot of permission to Name Node after you clear the perm changes.
>
> Na Li wrote:
> Another scenario to take care is that: 1) Name Node has synced with full snapshot before HDFS sync is disabled and is asking for seqNum = 1 after HDFS sync is enabled. A full snapshot should be sent to Name Node, but based on above logic, when there is no perm changes, empty list is sent back.
Enabling/disabling HDFS sync is done by changing an bunch of properties both in sentry server and namnode. When even it happens both sentry and namenode are restarted.
When ever namenode starts, it starts from fresh and the seqNum will be 0.
I'm dropping your comment. If you don't agree, plese re-open it with your comment.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review196971
-----------------------------------------------------------
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 5:02 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 125 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line125>
> >
> > Should you update the following condition for DBUpdateForwarder.getAllUpdatesFrom()?
> >
> > Replace
> > if (seqNum > curSeqNum) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > }
> >
> > With
> > if (seqNum > curSeqNum) {
> > if(seqNum = curSeqNum + 1) {
> > // No new notifications were processed.
> > return Collections.emptyList();
> > } else {
> > // perm change has been reset, get full perm snapshot
> > List<K>fullImage = retrieveFullImage();
> > //Only log if we have received full image
> > if( !fullImage.isEmpty()) {
> > LOGGER.info("({}) A newer full update with image number {} was found and being sent to HDFS",
> > retrieverType, curImgNum);
> > }
> > return fullImage;
> > }
> > }
> >
> > Once you clear the perm change, the seqNum from Name Node will be larger than curSeqNum. The correct behavior should be to send full snapshot of permission to Name Node after you clear the perm changes.
Another scenario to take care is that: 1) Name Node has synced with full snapshot before HDFS sync is disabled and is asking for seqNum = 1 after HDFS sync is enabled. A full snapshot should be sent to Name Node, but based on above logic, when there is no perm changes, empty list is sent back.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review196971
-----------------------------------------------------------
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review196971
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 125 (patched)
<https://reviews.apache.org/r/65533/#comment276976>
Should you update the following condition for DBUpdateForwarder.getAllUpdatesFrom()?
Replace
if (seqNum > curSeqNum) {
// No new notifications were processed.
return Collections.emptyList();
}
With
if (seqNum > curSeqNum) {
if(seqNum = curSeqNum + 1) {
// No new notifications were processed.
return Collections.emptyList();
} else {
// perm change has been reset, get full perm snapshot
List<K>fullImage = retrieveFullImage();
//Only log if we have received full image
if( !fullImage.isEmpty()) {
LOGGER.info("({}) A newer full update with image number {} was found and being sent to HDFS",
retrieverType, curImgNum);
}
return fullImage;
}
}
Once you clear the perm change, the seqNum from Name Node will be larger than curSeqNum. The correct behavior should be to send full snapshot of permission to Name Node after you clear the perm changes.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 596 (patched)
<https://reviews.apache.org/r/65533/#comment276977>
This method is not just for test, called in HMSFollower line 122. Why do you have this comment "the method only for test"?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 615 (patched)
<https://reviews.apache.org/r/65533/#comment276978>
it is called in HMSFollower, why comments "the method only for test"?
- Na Li
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 205 (original), 212-213 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line212>
> >
> > Isn't the new method name more confusing than before? isFullSnapshotRequired() means Sentry requires a full HMS snapshot while isReSyncRequired() sounds like Sentry requires to sync with HDFS, but the hdfsSyncEnable after this contradicts the name, isn't it?
>
> kalyan kumar kalvagadda wrote:
> synching in HMSFollower meaning synchornizing with HMS. Defination of being synchronized with HMS differs when HDFS sync feature is enabled OR disable.
I think we should keep the isFullSnapshotRequired() name as the meaning makes more sense for what it does 'see if a full HMS snapshot is required'. isReSyncRequired() could conflict with HDFS sync meaning which they're different.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 217-220 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line217>
> >
> > This needs a better explanation. I'm trying to understand the reason of these lines. A re-sync with HDFS needed? But HDFS is disabled, right?
>
> kalyan kumar kalvagadda wrote:
> re-synching in HMSFollower meaning synchornizing with HMS.
> Let's hdfs sync is disabled and sentry detects that it is out of sync, it should set the last processed eventid as 0 and process all the notification event in HMS NOTIFICATION_LOG table.
I see that there is an if (hdfsSyncEnabled) inside the createFullSnapshot() method. Can we put this block inside the createFullSnapshot() method instead?
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 254 (original), 274 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line274>
> >
> > Why do we need a method change? Is this resync meant for hdfs or hms full snapshot?
>
> kalyan kumar kalvagadda wrote:
> Idea of this patch is to avaoid getting full snapshot when hdfs sync is not enabled.
>
> method isFullSnapshotRequired, tells us if sentry is out-of-sync with HMS.
>
> isFullSnapshotRequired is not correct. Please suggest if "isReSyncRequired" is not reasonable.
isFullSnapshopt() is more reasonable. isReSyncRequired() conflicts with areNotificationOutOfSync() and Hdfs Sync.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197051
-----------------------------------------------------------
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 205 (original), 212-213 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line212>
> >
> > Isn't the new method name more confusing than before? isFullSnapshotRequired() means Sentry requires a full HMS snapshot while isReSyncRequired() sounds like Sentry requires to sync with HDFS, but the hdfsSyncEnable after this contradicts the name, isn't it?
synching in HMSFollower meaning synchornizing with HMS. Defination of being synchronized with HMS differs when HDFS sync feature is enabled OR disable.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 217-220 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line217>
> >
> > This needs a better explanation. I'm trying to understand the reason of these lines. A re-sync with HDFS needed? But HDFS is disabled, right?
re-synching in HMSFollower meaning synchornizing with HMS.
Let's hdfs sync is disabled and sentry detects that it is out of sync, it should set the last processed eventid as 0 and process all the notification event in HMS NOTIFICATION_LOG table.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Lines 235 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line235>
> >
> > Doesn't this required notificationId = ... like the previous isResyncRequired call?
nope. in previous case, notificationId is used to fetch the notifications but in this case control will would return. In the subsequent run the notificationId is fetched from the database again.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 254 (original), 274 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953486#file1953486line274>
> >
> > Why do we need a method change? Is this resync meant for hdfs or hms full snapshot?
Idea of this patch is to avaoid getting full snapshot when hdfs sync is not enabled.
method isFullSnapshotRequired, tells us if sentry is out-of-sync with HMS.
isFullSnapshotRequired is not correct. Please suggest if "isReSyncRequired" is not reasonable.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Line 62 (original), 62 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953488#file1953488line62>
> >
> > Avoid using the * when importing packages.
will change.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Lines 354 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953488#file1953488line357>
> >
> > 'i stead' should be 'instead'
will change.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Lines 355 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953488#file1953488line358>
> >
> > What will it prevent?
will update.
> On Feb. 7, 2018, 10:14 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Lines 486-488 (patched)
> > <https://reviews.apache.org/r/65533/diff/1/?file=1953488#file1953488line489>
> >
> > I see this block repeated several times, can we create a method for it?
will change
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197051
-----------------------------------------------------------
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197051
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Line 205 (original), 212-213 (patched)
<https://reviews.apache.org/r/65533/#comment277150>
Isn't the new method name more confusing than before? isFullSnapshotRequired() means Sentry requires a full HMS snapshot while isReSyncRequired() sounds like Sentry requires to sync with HDFS, but the hdfsSyncEnable after this contradicts the name, isn't it?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 217-220 (patched)
<https://reviews.apache.org/r/65533/#comment277151>
This needs a better explanation. I'm trying to understand the reason of these lines. A re-sync with HDFS needed? But HDFS is disabled, right?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 235 (patched)
<https://reviews.apache.org/r/65533/#comment277152>
Doesn't this required notificationId = ... like the previous isResyncRequired call?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Line 254 (original), 274 (patched)
<https://reviews.apache.org/r/65533/#comment277153>
Why do we need a method change? Is this resync meant for hdfs or hms full snapshot?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Line 62 (original), 62 (patched)
<https://reviews.apache.org/r/65533/#comment277154>
Avoid using the * when importing packages.
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 354 (patched)
<https://reviews.apache.org/r/65533/#comment277156>
'i stead' should be 'instead'
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 355 (patched)
<https://reviews.apache.org/r/65533/#comment277157>
What will it prevent?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 486-488 (patched)
<https://reviews.apache.org/r/65533/#comment277155>
I see this block repeated several times, can we create a method for it?
- Sergio Pena
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Xinran Tinney <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197329
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 451 (patched)
<https://reviews.apache.org/r/65533/#comment277469>
2 empty lines
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 487 (patched)
<https://reviews.apache.org/r/65533/#comment277470>
can this string be created as a string variable so no need to hard coded
- Xinran Tinney
On Feb. 6, 2018, 7:06 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2018, 7:06 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
>
>
> Diff: https://reviews.apache.org/r/65533/diff/1/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Feb. 14, 2018, 10:04 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 256 (original), 280 (patched)
> > <https://reviews.apache.org/r/65533/diff/2/?file=1960614#file1960614line280>
> >
> > the comment needs to be changed "Create Full HMS snapshot" to "Need to re-sync"
will fix.
> On Feb. 14, 2018, 10:04 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 264 (original), 288 (patched)
> > <https://reviews.apache.org/r/65533/diff/2/?file=1960614#file1960614line288>
> >
> > same thing here
will fix.
> On Feb. 14, 2018, 10:04 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 3110 (original), 3126 (patched)
> > <https://reviews.apache.org/r/65533/diff/2/?file=1960616#file1960616line3126>
> >
> > the name of the function does not match what it is doing. Can you change the function name to "isAuthzPathsMappingEmpty"?
The name of the table where we store the snapshot information is named as MAuthzPathsMapping. I think changing it to isAuthzPathsMappingEmpty would be confusing.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197582
-----------------------------------------------------------
On Feb. 14, 2018, 2:53 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2018, 2:53 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/2/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197582
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Line 256 (original), 280 (patched)
<https://reviews.apache.org/r/65533/#comment277787>
the comment needs to be changed "Create Full HMS snapshot" to "Need to re-sync"
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Line 264 (original), 288 (patched)
<https://reviews.apache.org/r/65533/#comment277788>
same thing here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3110 (original), 3126 (patched)
<https://reviews.apache.org/r/65533/#comment277789>
the name of the function does not match what it is doing. Can you change the function name to "isAuthzPathsMappingEmpty"?
- Na Li
On Feb. 14, 2018, 2:53 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2018, 2:53 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/2/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
> > Line 261 (original), 279 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962035#file1962035line279>
> >
> > isAuthzPathsSnapshotEmpty() checks if MAuthzPathsSnapshotId is empty, why did you change it to MAuthzPathsMapping?
MAuthzPathsMapping is nothing but snapshot. It just that naming of MAuthzPathsMapping class is confusing.
> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 587-589 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962037#file1962037line587>
> >
> > What about MAuthzPathsSnapshotId.class?
Data in MAuthzPathsSnapshotId.class is not cleared intentionally. This data will help sentry retain the history of snapshots taken before
and help in picking appropriate ID even when hdfs sync is enabled/disabled. I have added the same in the code.
> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 3115 (original), 3131 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962037#file1962037line3131>
> >
> > The name of the method and what is checks does not match. Why is it needed to check if the MAuthzPathsMapping is empty instead of the MAuthzPathsSnapshotId?
MAuthzPathsMapping is nothing but snapshot. It just that naming of MAuthzPathsMapping class is confusing.
> On Feb. 20, 2018, 9:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Lines 1148-1152 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962039#file1962039line1160>
> >
> > What's wrong with /* */?
There is nothing wrong. I have updated the test case and have updated the java doc for the same. I just changed the way it is commented.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197810
-----------------------------------------------------------
On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 9 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197810
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 125-126 (patched)
<https://reviews.apache.org/r/65533/#comment278088>
Make sure to add an statement about what users should do in case this scenario happens.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 223 (patched)
<https://reviews.apache.org/r/65533/#comment278089>
Can we avoid this extra call by setting the notificationId to 0? The setLastProcessedNotificatioNID already persists the ID to 0, so I don't see a necessary getLastProcessedNotificatoiNID call.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Line 261 (original), 279 (patched)
<https://reviews.apache.org/r/65533/#comment278090>
isAuthzPathsSnapshotEmpty() checks if MAuthzPathsSnapshotId is empty, why did you change it to MAuthzPathsMapping?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
Lines 307 (patched)
<https://reviews.apache.org/r/65533/#comment278091>
Exception if an error occurs attempting to fetch the latest notification ID from the HMS
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 587-589 (patched)
<https://reviews.apache.org/r/65533/#comment278092>
What about MAuthzPathsSnapshotId.class?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3115 (original), 3131 (patched)
<https://reviews.apache.org/r/65533/#comment278093>
The name of the method and what is checks does not match. Why is it needed to check if the MAuthzPathsMapping is empty instead of the MAuthzPathsSnapshotId?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 520-522 (patched)
<https://reviews.apache.org/r/65533/#comment278094>
Should you delete these comments?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 584-587 (patched)
<https://reviews.apache.org/r/65533/#comment278095>
Should you delete these comments?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 1148-1152 (patched)
<https://reviews.apache.org/r/65533/#comment278097>
What's wrong with /* */?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 997-1000 (original), 1159-1162 (patched)
<https://reviews.apache.org/r/65533/#comment278096>
Should you delete these comments?
- Sergio Pena
On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 9 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Feb. 20, 2018, 8:28 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Line 336 (original), 354 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962039#file1962039line359>
> >
> > why do you comment out this line?
This is not fixed. Did you upload the latest patch?
> On Feb. 20, 2018, 8:28 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
> > Line 999 (original), 1161 (patched)
> > <https://reviews.apache.org/r/65533/diff/3/?file=1962039#file1962039line1173>
> >
> > if you don't need those lines, can you just remove them?
This is not fixed. Did you upload the latest patch?
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197804
-----------------------------------------------------------
On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 9 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review197804
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Lines 306 (patched)
<https://reviews.apache.org/r/65533/#comment278082>
should be "fullsnapshot should not have been fetched"
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Line 336 (original), 354 (patched)
<https://reviews.apache.org/r/65533/#comment278083>
why do you comment out this line?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
Line 999 (original), 1161 (patched)
<https://reviews.apache.org/r/65533/#comment278084>
if you don't need those lines, can you just remove them?
- Na Li
On Feb. 15, 2018, 9 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 9 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/#review198073
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Feb. 21, 2018, 6:45 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65533/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2018, 6:45 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-2115
> https://issues.apache.org/jira/browse/SENTRY-2115
>
>
> Repository: sentry
>
>
> Description
> -------
>
> **Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
> **Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
> **Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
> **New Behavior:** Full snapshots need not be taken in all
> When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
>
> **Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
> **New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
> To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
>
>
> Diff: https://reviews.apache.org/r/65533/diff/4/
>
>
> Testing
> -------
>
> Made sure that all the tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/
-----------------------------------------------------------
(Updated Feb. 21, 2018, 6:45 p.m.)
Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
Changes
-------
Lina,
It was my issue. I did not updated the patch properly.
Bugs: SENTRY-2115
https://issues.apache.org/jira/browse/SENTRY-2115
Repository: sentry
Description
-------
**Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
**Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
**Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
**New Behavior:** Full snapshots need not be taken in all
When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
**Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
**New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
Diff: https://reviews.apache.org/r/65533/diff/4/
Changes: https://reviews.apache.org/r/65533/diff/3-4/
Testing
-------
Made sure that all the tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/
-----------------------------------------------------------
(Updated Feb. 15, 2018, 9 p.m.)
Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
Changes
-------
Addressed comments from sergio and lina.
Bugs: SENTRY-2115
https://issues.apache.org/jira/browse/SENTRY-2115
Repository: sentry
Description
-------
**Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
**Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
**Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
**New Behavior:** Full snapshots need not be taken in all
When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
**Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
**New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
Diff: https://reviews.apache.org/r/65533/diff/3/
Changes: https://reviews.apache.org/r/65533/diff/2-3/
Testing
-------
Made sure that all the tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 65533: SENTRY-2115: Incorrect behavior of
HMsFollower when HDFSSync feature is disabled.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65533/
-----------------------------------------------------------
(Updated Feb. 14, 2018, 2:53 p.m.)
Review request for sentry, Alexander Kolbasov, Na Li, and Sergio Pena.
Changes
-------
Addressed comments and also added e2e tests to test the behavior.
Bugs: SENTRY-2115
https://issues.apache.org/jira/browse/SENTRY-2115
Repository: sentry
Description
-------
**Scenario-1:** When HDFS sync is disabled, and sentry is started for the first time.
**Scenario-2:** When HDFS sync is disabled, and current event-id from HMS is less than last event-d processed by sentry
**Scenario-3:** When HDFS sync is disabled, and first event-id in the subsequent pull is not greater than the last event-id processed by sentry by 1.
**New Behavior:** Full snapshots need not be taken in all
When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
**Scenario-4:** Initially HDFS sync was enabled and later disabled for while and then HDFS sync is enabled and sentry service is restarted to get it to effect.
**New Behavior:** When Sentry detects out-of-sync situations, it should reset SENTRY_HMS_NOTIFICATION_ID table and start processing the event in HMS_NOTIFICATION_LOG from beginning.
To handle scenario explained in Scenario-4, sentry should reset the mapping information when ever HDFS sync is disabled. That way it can learn from scratch when the feature is enabled back. There is no value is holding stale data even when we know it will have issues when the feature is enabled back.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
Diff: https://reviews.apache.org/r/65533/diff/2/
Changes: https://reviews.apache.org/r/65533/diff/1-2/
Testing
-------
Made sure that all the tests passed.
Thanks,
kalyan kumar kalvagadda