----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57219/#review167918 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java Lines 134 (patched) <https://reviews.apache.org/r/57219/#comment239891> currentId can *increase* between get() calls. This call isn't an optimization - it is critical for correctness. Otherwise you may get into a situation when waiter will be never waken up. For example, you checked for the condition at the top and right after the check counter increased. You place the waiter in the queue, but it may never be waken up since the increase already happened. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java Lines 31 (patched) <https://reviews.apache.org/r/57219/#comment239894> It is rather tricky to test for this reliably - what would the test actually do? Note that subsequent updates will wakeup all previous waiters so this may cover up any race conditions. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java Lines 55 (patched) <https://reviews.apache.org/r/57219/#comment239898> This is a test, so I want to keep it simple. We could use the latch but then we need to catch exceptions, etc. But we can, in fact, wait untill we have enough waiters. I'll change the test to do that. - Alexander Kolbasov On March 3, 2017, 3:08 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57219/ > ----------------------------------------------------------- > > (Updated March 3, 2017, 3:08 a.m.) > > > Review request for sentry, Misha Dmitriev, Lei Xu, Hao Hao, kalyan kumar > kalvagadda, Mat Crocker, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1601 > https://issues.apache.org/jira/browse/SENTRY-1601 > > > Repository: sentry > > > Description > ------- > > SENTRY-1601 Implement HMS Notification barrier on the server side > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java > 3f7542c2b7bfacc99b5a2bed14f841d5ec89f6e1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 30e91ae05af5350ea30f08e099d73feac94c120a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/57219/diff/5/ > > > Testing > ------- > > The new CounterWait class has a unit test. End to end functionality will be > tested once HMS part is implemented. > > > Thanks, > > Alexander Kolbasov > >
