----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60669/#review180196 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 547 (patched) <https://reviews.apache.org/r/60669/#comment255230> This can be protected? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 548 (patched) <https://reviews.apache.org/r/60669/#comment255231> Incorrect indentation sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 552 (patched) <https://reviews.apache.org/r/60669/#comment255232> Move to the place where it is used or just put `lastNotificationID - changesToKeep` directly instead of maxIDDeleted Also name is difficult to read. May be use maxDeleted if you want to keep it. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 562 (patched) <https://reviews.apache.org/r/60669/#comment255233> Use built-in formatting with {} and make this debug instead of info sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 594 (patched) <https://reviews.apache.org/r/60669/#comment255234> Since it is configurable, makes sense to document the configuration string (`sentry.server.delta.keep.count`) sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 599 (patched) <https://reviews.apache.org/r/60669/#comment255235> debug? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 607 (patched) <https://reviews.apache.org/r/60669/#comment255236> This doesn't add any value - probably should be removed. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2937 (patched) <https://reviews.apache.org/r/60669/#comment255237> This is test-only, so can be protected - Alexander Kolbasov On July 8, 2017, 3:31 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60669/ > ----------------------------------------------------------- > > (Updated July 8, 2017, 3:31 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1762 > https://issues.apache.org/jira/browse/SENTRY-1762 > > > Repository: sentry > > > Description > ------- > > SENTRY_HMS_NOTIFICATION_ID table will be updated with the ID's for every > notification that sentry processes. This data grows with time. > we need to periodically purge this data as it will effect the performance. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 1402ab1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > ec938da > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 83f00ca > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > ac266fe > > > Diff: https://reviews.apache.org/r/60669/diff/2/ > > > Testing > ------- > > Added new tests to test the code change added. > > > Thanks, > > kalyan kumar kalvagadda > >
