> On April 14, 2017, 6:45 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2535 (patched) > > <https://reviews.apache.org/r/58412/diff/1/?file=1691537#file1691537line2535> > > > > could the exception type to be more specific? Such as noSuchPath or its > > parent exception?
We are calling TransactionBlock.execute which throws 'Exception'. Given this cannot see a way to avoid it. > On April 14, 2017, 6:45 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2562 (patched) > > <https://reviews.apache.org/r/58412/diff/1/?file=1691537#file1691537line2562> > > > > should we continue delete the path, and then throw the exceptions? > > Those changes work in Hive, so Sentry needs to be more tolerate to errors. Relaxed the checking here. > On April 14, 2017, 6:45 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2579 (patched) > > <https://reviews.apache.org/r/58412/diff/1/?file=1691537#file1691537line2579> > > > > If deleting one entry fails, should the operation continues for the > > rest of entries before throwing exception, or throw exception right away? > > > > The changes are from Hive, and they succeeds at Hive. If exception > > happens at Sentry, should we be more tolenrate on the error? I have updated it to handle one authzObjs only now. > On April 14, 2017, 6:45 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2646 (patched) > > <https://reviews.apache.org/r/58412/diff/1/?file=1691537#file1691537line2646> > > > > what is the sequence when renaming happens? Will the old path renamed > > to new path, and then rename authz path mapping? > > > > If that is the case, then "mOldPath == null" should be allowed. We delete the old path first and add the new path. I also relax the checking here. > On April 14, 2017, 6:45 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Line 2473 (original), 2661 (patched) > > <https://reviews.apache.org/r/58412/diff/1/?file=1691537#file1691537line2661> > > > > should we throw exception for mOldPath == null? Is that possible that > > old path is renamed, and then this function is called? yeah, I have relax the checking here. - Hao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58412/#review172007 ----------------------------------------------------------- On April 18, 2017, 7:34 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58412/ > ----------------------------------------------------------- > > (Updated April 18, 2017, 7:34 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > SENTRY-1587: Refactor SentryStore transaction to persist a single path > transcation bundled with corresponding delta path change > > Change-Id: Ice77f631e92c0e74b42ae644e1b2f90c6089e62f > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > 90aaaef0e15306627d7108f12a74a29848055c0b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 14e967aa1065f16e8d4c3f61db2f9055959fa9e6 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > c22364fa26c80415827313f4d26f6c53b71b6f6c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 08520f30f3529af88d7dab9ae3623f28f0e43558 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 16676fb13b0d5015aefe892a6f7e46812ea75124 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 87e329588990be129197d598dd1db4487f8e0f25 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 24ab1a8b392f23bc75759733bef7cecd4bc2ac34 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 7769f24bb4c062016084bcafe7d50a0f0e013824 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 0e97466c9ba6973d8e787819e292c3b826472b38 > > > Diff: https://reviews.apache.org/r/58412/diff/2/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
