----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58975/#review174243 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryLastNotification.java Lines 21 (patched) <https://reviews.apache.org/r/58975/#comment247360> Representation of a single-row table containing the most up-to date value of the HMS notification ID that was processed by the Sentry server. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryLastNotification.java Lines 25 (patched) <https://reviews.apache.org/r/58975/#comment247361> This instance is updated sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryLastNotification.java Lines 45 (patched) <https://reviews.apache.org/r/58975/#comment247362> Why do you need all this? Can't you just use notificationId as the hashCode? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo Lines 304 (patched) <https://reviews.apache.org/r/58975/#comment247367> What is htis column? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java Lines 92 (patched) <https://reviews.apache.org/r/58975/#comment247372> Why do you need to pass lastNtification of you are not using it to read the value and just modifying the value anyway? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java Lines 96 (patched) <https://reviews.apache.org/r/58975/#comment247373> What if there was another update that hapened in between? Will you overwrite the value? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 126 (patched) <https://reviews.apache.org/r/58975/#comment247368> This is never used - Alexander Kolbasov On May 3, 2017, 9:26 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58975/ > ----------------------------------------------------------- > > (Updated May 3, 2017, 9:26 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, > Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1669 > https://issues.apache.org/jira/browse/SENTRY-1669 > > > Repository: sentry > > > Description > ------- > > HMS follower periodically pulls new notifications from the HMS. It needs to > know the last notification id that sentry processed in order to send a > request to HMS asking for latest notifications. > Currently HMS follower is depending on the notification stores in > SENTRY_PATH_CHANGE table. This may not give the last notification processed > all the time. > > Let's take en example here > Notification ID Event MAX(Notification ID) in > SENTRY_PATH_CHANGE > 100 create table db1.tb1 100 > 101 drop table db1.tb1 99 > > After processing notification with ID 101 in above example, MAX(Notification > ID) in SENTRY_PATH_CHANGE would give you 99. If HMS follower depends on this > information, it has to process notifications 100 and 101 again. This is not > we want. > > Solution: > Store Last Notification ID in separate table and use it instead. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryLastNotification.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > 8fd5278 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java > f590a52 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > ef67865 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > ec8676e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java > 083e0ac > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > e7443eb > > > Diff: https://reviews.apache.org/r/58975/diff/1/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
