----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29041/#review68577 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java <https://reviews.apache.org/r/29041/#comment112901> I know you want to get HAContext via "synchronized static HAContext get()" instead of creating from constructor. But only remove public from constructor, we also can create HAContext via constructor in same package. We should change public to private. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LockingSentryStore.java <https://reviews.apache.org/r/29041/#comment112902> please put the license to the top. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LockingSentryStore.java <https://reviews.apache.org/r/29041/#comment112900> I think LockingSentryStore is used to decorate InMemSentryStore. So we'd better extends InMemSentryStore rather than implements SentryStore. This patch looks fine to me. A few comments, please feel free to fix them. - Xiaomeng Huang On 十二月 18, 2014, 8:04 a.m., Arun Suresh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29041/ > ----------------------------------------------------------- > > (Updated 十二月 18, 2014, 8:04 a.m.) > > > Review request for sentry, bc Wong, Lenni Kuff, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > This patch includes a base SentryStore Decorator (LockingSentryStore) that > adds locking to an existing SentryStore > It also inclues two implementations for the Decorator > 1) SentryStoreWithLocalLock uses the standard > java.util.concurrent.ReadWriteLock.. usefull for single instance deployments > of the Sentry Service > 2) SentryStoreWithDistributedLock implements a Distributed Lock. It uses the > Hazelcast (http://hazelcast.org) library. > More decription on the JIRA : issues.apache.org/jira/browse/SENTRY-567 > > TODO: Another possible implementation using Zookeeper. HazelCast and > Zookeeper has its own merits. Zookeeper might be better for global lock > consistency since it denies availability to peers that are not part of the > Quorum in the case of partition tolerence. Hazelcast chooses availibliity but > APIs are simpler than Zookeeper. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java > 523261e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LockingSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreWithDistributedLock.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreWithLocalLock.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29041/diff/ > > > Testing > ------- > > > Thanks, > > Arun Suresh > >
