> On 十二月 9, 2014, 10:09 a.m., Prasad Mujumdar wrote: > > The patch itself looks fine. > > > > I do have a bit of concern about mixing the ZK operation with DB > > transaction. This essentially makes the Sentry RPC a distributed > > transaction, between SentryStore and ZK. Since there no 2 phase commit, > > it's always possible to end up with local sequence number (the patch uses > > the local seq id if the ZK counter can't be obtained). > > What are your thoughts on getting the sequence from the database itself, eg > > using sequence object of a simple countered stored in the DB ?
Yes, I'm agreed with you, mixing the ZK operation with DB transaction may cause the problem of synchronization. And it's a good choice to store the counter in the DB. I will look into it and refactor the code. Thanks a lot for helping to point out it and give me your good suggestion. - Dapeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25981/#review64322 ----------------------------------------------------------- On 十月 27, 2014, 2:54 p.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25981/ > ----------------------------------------------------------- > > (Updated 十月 27, 2014, 2:54 p.m.) > > > Review request for sentry, Arun Suresh, Lenni Kuff, Prasad Mujumdar, and > Sravya Tirukkovalur. > > > Bugs: SENTRY-457 > https://issues.apache.org/jira/browse/SENTRY-457 > > > Repository: sentry > > > Description > ------- > > As **CommitContext** contains a sequenceId, it will auto-increment per > operation, In HA environment, we should make it work, also add a distributed > counter > * Add **CommitContextFactory** , use **getCommitContext(UUID serverUUID, long > sequenceId)** in the factory instead of **new CommitContext(UUID serverUUID, > long sequenceId)** > * **counter.getGlobalSequenceId()** will return the Global sequence id > ````java > public CommitContext getCommitContext(UUID serverUUID, long sequenceId) { > if (haEnabled) { > try { > return new CommitContext(serverUUID, counter.getGlobalSequenceId()); > } catch (Exception e) { > LOGGER.error("Error in get global sequence Id, return the sequenceId > in service!" ,e); > return new CommitContext(serverUUID, sequenceId); > } > } else { > return new CommitContext(serverUUID, sequenceId); > } > } > ```` > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContextFactory.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 017cf08 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > b54e12e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 52eaeed > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryDistributeCounter.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25981/diff/ > > > Testing > ------- > > Unit Test passed in local > > > Thanks, > > Dapeng Sun > >
