----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59174/#review175558 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java Lines 111 (patched) <https://reviews.apache.org/r/59174/#comment248950> All our unit test methods start with 'test...'. We should keep this convention as well. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java Lines 143 (patched) <https://reviews.apache.org/r/59174/#comment248955> I wonder why we need to test the whole processNotificationEvents() in parallel. Are we trying to validate that sentryStore.persistLastProcessedNotificationID() can run in parallel? Why not run the tests on that method instead? Or, why not running processNotificationEvents() twice in serial? That I think will do the same test if you want to validate that current data cannot be altered any more. The HMSFollower.processNotificationEvents() has a lot of code that is not affected by parallelism. I think we should isolate the concurrent code and test only that if necessary. Btw, doing tests with multi threads is a tough challenge. - Sergio Pena On May 18, 2017, 7:16 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59174/ > ----------------------------------------------------------- > > (Updated May 18, 2017, 7:16 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1761 > https://issues.apache.org/jira/browse/SENTRY-1761 > > > Repository: sentry > > > Description > ------- > > Added test to make sure that SENTRY_HMS_NOTIFICATION_ID table is properly > updated even when multiple HMSFollowers are processing notifications from HMS. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/59174/diff/4/ > > > Testing > ------- > > > Thanks, > > kalyan kumar kalvagadda > >
