----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57219/#review167778 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java Lines 137 (patched) <https://reviews.apache.org/r/57219/#comment239740> Hmm ... imagine the following sequence: 1. Threads 1 and 2 concurrently call waitFor() with the same value, and both call waiters.put(eid). 2. Thread 1 checks that the currentId is still less than the value, and proceeds to eid.waitFor(). 3. Update thread calls update(), increments currentId but does not yet call wakeup(newValue). 4. Thread 2 checks currentId to be equal or greater than the value, and proceeds with waiters.remove(eid). Since ValueEvent.equals() is based on the value, not object identity, it may remove the wrong ValueEvent object added by thread 1. 5. Update thread calls wakeup() but the ValueEvent object of thread 1 is not in the queue. 6. Thread 1 blocks on eid.waitFor() forever. Am I missing something? - Vadim Spector On March 2, 2017, 5:26 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57219/ > ----------------------------------------------------------- > > (Updated March 2, 2017, 5:26 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/4/ > > > Testing > ------- > > The new CounterWait class has a unit test. End to end functionality will be > tested once HMS part is implemented. > > > Thanks, > > Alexander Kolbasov > >
