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