> On March 2, 2017, 1:01 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > > Lines 89 (patched) > > <https://reviews.apache.org/r/57219/diff/1/?file=1652935#file1652935line89> > > > > Nit: why spread this over 3 lines if you can fit it in two?
To avoid very long lines > On March 2, 2017, 1:01 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java > > Lines 208 (patched) > > <https://reviews.apache.org/r/57219/diff/1/?file=1652936#file1652936line208> > > > > I guess you tested this code and it works correctly, but it seems a bit > > confusing/inelegant. I wonder if java.util.concurrent.Semaphore initialized > > with 1 is more appropriate for this purpose. What do you think? This is, essentially message-based synchronization - I am using a blocking queue as a messaging interface. I guess it is possible to achieve the same purpose with Semaphore as well. I prototyped it and it seems ok as well, probably simple enough for this case. will change. > On March 2, 2017, 1:01 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java > > Lines 282 (patched) > > <https://reviews.apache.org/r/57219/diff/1/?file=1652936#file1652936line282> > > > > I think the methods of this class will be easier to understand if the > > fields, traditionally, are declared at the top of the class rather than at > > the bottom. I wanted to put public interfaces at the top, but will move if this makes things easier. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57219/#review167618 ----------------------------------------------------------- On March 1, 2017, 9:32 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57219/ > ----------------------------------------------------------- > > (Updated March 1, 2017, 9:32 p.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/1/ > > > Testing > ------- > > The new CounterWait class has a unit test. End to end functionality will be > tested once HMS part is implemented. > > > Thanks, > > Alexander Kolbasov > >
