----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58975/#review174415 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java Lines 91 (patched) <https://reviews.apache.org/r/58975/#comment247548> do you need to catch exception here and don't allow it to propergate to caller? if two threads are updating with the same notification Id, one will fail. But this failure is acceptable sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 3419 (patched) <https://reviews.apache.org/r/58975/#comment247552> We should remove old rows when the table size exceeds certain number. Otherwise, max(notificationId) will take longer and longer sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 3423 (patched) <https://reviews.apache.org/r/58975/#comment247550> should this be "pm.makePersistent(new MSentryHmsNotification(notificationId));"? Otherwise, how can pm knows what class it needs to update? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Line 281 (original), 303 (patched) <https://reviews.apache.org/r/58975/#comment247549> should you change "CurrentEventID to "currentNotificationID to be consistent? - Na Li On May 10, 2017, 12:34 a.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58975/ > ----------------------------------------------------------- > > (Updated May 10, 2017, 12:34 a.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 > ------- > > Sentry should store Id's of all the notifications that it processed in a > seperate database. > HMSFollower before every run should get the last notification ID from this > persistent store. > > > Diffs > ----- > > > 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 > e271ea4 > > 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/2/ > > > Testing > ------- > > Sentry stores Id's of all the notifications that it processed in a seperate > database. > HMSFollower before every run gets the last notification ID from this > persistent store. > > > Thanks, > > kalyan kumar kalvagadda > >
