> 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 > >
