> On April 10, 2017, 9:25 a.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java > > Lines 87 (patched) > > <https://reviews.apache.org/r/58281/diff/1/?file=1686601#file1686601line87> > > > > Is it possible for the following scenario to happen? > > > > There are two Sentry server instances running. Request A goes to Sentry > > A, and add a perm update. At the same time HMSFollower at Sentry B gets a > > path update. > > > > Both Sentry A and Sentry B calls this function to persist update. And > > both get lastChangeID to be, for example, 3. If Sentry A calls > > "pm.makePersistent" first, the "SentryStore.getLastProcessedChangeIDCore" > > is 4 now. Then Sentry B calls "pm.makePersistent", and the > > "SentryStore.getLastProcessedChangeIDCore" remains as 4, but it should be 5 > > as a new update is persisted (will this cause exception as two update > > entries are having the same changeID?).
`jdo.makePersistent` is doing `INSERT`. And since `changeID` is the primary ID, only one of the insertation can success. The other one will go the retry in the transaction and pick up the next change ID to persist. > On April 10, 2017, 9:25 a.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > > Lines 2581 (patched) > > <https://reviews.apache.org/r/58281/diff/1/?file=1686603#file1686603line2581> > > > > did you reproduce the original issue using this test code? If not, then > > it cannot verify the fix for the original issue. Yes, it did. - Lei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58281/#review171444 ----------------------------------------------------------- On April 7, 2017, 5:38 p.m., Lei Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58281/ > ----------------------------------------------------------- > > (Updated April 7, 2017, 5:38 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-1643 > https://issues.apache.org/jira/browse/SENTRY-1643 > > > Repository: sentry > > > Description > ------- > > When it relies on the SQL auto increment primary key as ChangeID, it can not > guarentee the consectivity of the IDs, because while each transaction claims > new ID from the table counter, the concurrent transactions which were not > success would not return the claimed changeIDs to the pool, thus it can not > guarateen the consectivity of change IDs. > > This patch changes to use application logic to force the consectivity of IDs. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java > a0d34459d7b2f70e863ef6e078401df81381c91b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java > 476fbcb2ad26de23757842111beb12b154e1562b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DeltaTransactionBlock.java > f590a5296c047e1acedd39a4f2e4f1de98008d32 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 802b9c6cbf8e9ad015e37037b809b58c956de746 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > aaa0b9fd30bb68fded67f885af4f77bc71398e77 > > > Diff: https://reviews.apache.org/r/58281/diff/1/ > > > Testing > ------- > > Add a new test to conurrently insert changes. > > mvn test -Dtest=TestSentryStore. > > > Thanks, > > Lei Xu > >
