> On Aug. 16, 2017, 8:18 p.m., Na Li wrote: > > I only see change to make notificationID not primary key for its table. I > > don't see the code to go back time and fetch updates with lower > > notificationID. Are they included in this patch? > > Na Li wrote: > also, when you get changes with old notificiation ID, you need to apply > all of them. For example, you got [1,2,3,4] before, not you got [2,3,3,4]. > The second 3 is different form the first 3. the '4' is the same as before. > You should apply [3,4] (the second 3 and first 4). if you only apply the > second 3, your actual applying order will be [1,2,3,4,3], which is not > correct if the result is determined by the SQL execution order, and which is > captured in notificationID order.
This patch doesn't do that. This patch only allows Sentry to handle events with duplicated IDs. The issue you mention is a different issue that we need to handle in a different way (as you mentioning) > On Aug. 16, 2017, 8:18 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/resources/008-SENTRY-1569.derby.sql > > Line 23 (original) > > <https://reviews.apache.org/r/61696/diff/1/?file=1798826#file1798826line23> > > > > Do we query the entries based on notificationID? if so, should we still > > create index for notificationID (not unique index)? We don't for SENTRY_PATH_CHANGE. In fact, I created this jira https://issues.apache.org/jira/browse/SENTRY-1885 to removed this unused NOTIFICATION_ID. - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61696/#review183063 ----------------------------------------------------------- On Aug. 16, 2017, 7:41 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61696/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2017, 7:41 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, and Vamsee > Yarlagadda. > > > Bugs: sentry-1803 > https://issues.apache.org/jira/browse/sentry-1803 > > > Repository: sentry > > > Description > ------- > > This patch made changes on the SENTRY_PATH_CHANGES and > SENTRY_HMS_NOTIFICATION_ID to allow duplicated IDs so that we handle HMS > notifications with same event IDs correctly. > > It also adds a few test cases to check that duplicated events IDs are handled > correctly. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > e8982f62b51d2db98ff280a42d01ad22333fd4ec > > sentry-provider/sentry-provider-db/src/main/resources/008-SENTRY-1569.derby.sql > 2a017ff069e7bd9822258b59a8bb9ea8f0049c44 > > sentry-provider/sentry-provider-db/src/main/resources/008-SENTRY-1569.mysql.sql > b587e40ac106071e742343423d9d7a52ebed45c7 > > sentry-provider/sentry-provider-db/src/main/resources/008-SENTRY-1569.oracle.sql > 860f99269d294c66c136ab2e575065458e31992d > > sentry-provider/sentry-provider-db/src/main/resources/008-SENTRY-1569.postgres.sql > c7c38e3b7201398287fcdadb605639f8b4f0b02a > sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-2.0.0.sql > 01be5098f3ba44624896c3668dad3dd6342ff1f4 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-2.0.0.sql > dca9e47bb2c7afab88e14da68f6f1425f687c117 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-2.0.0.sql > 0189724d9059639aa1ccce38c0793c9df1c87989 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-2.0.0.sql > ecb76fc656a4fde4b5874df4b22660acd30b52bd > > sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-2.0.0.sql > 3e5d554cbf716ef72c341be7d76eb78982ba0303 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-upgrade-db2-1.7.0-to-2.0.0.sql > ac85968ebd2efbd7652ec45f8a2ddfa3398e7e48 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > d35cafbd6189986d51221d867437532701813612 > > > Diff: https://reviews.apache.org/r/61696/diff/1/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Sergio Pena > >
