> On Oct. 22, 2018, 6:52 p.m., Arjun Mishra wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 5110 (patched) > > <https://reviews.apache.org/r/68973/diff/5/?file=2101805#file2101805line5121> > > > > We should probably replace "executeTransactionWithRetry" and > > "executeTransaction". We don't want to get into a weird state where we have > > thrown the exception but the transaction retries. However we have kicked > > off a new persist job in the original transaction
We don't want to retry snapshot completety when there is an issue with one path. Having retry in SnapshotUpdater would avaoid that. SnapshotUpdater would report failure only when there is a failure after all the retries. > On Oct. 22, 2018, 6:52 p.m., Arjun Mishra wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 5111 (patched) > > <https://reviews.apache.org/r/68973/diff/5/?file=2101805#file2101805line5122> > > > > Are we here creating a single transaction for every persist? If so, > > will this have performance impact? we are creating new transaction to persist all the paths associated to an object. This object could be a database or a table. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68973/#review209892 ----------------------------------------------------------- On Oct. 22, 2018, 2:42 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68973/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2018, 2:42 p.m.) > > > Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena. > > > Bugs: SENTRY-2305 > https://issues.apache.org/jira/browse/SENTRY-2305 > > > Repository: sentry > > > Description > ------- > > I have considered multiple options. Persisting in batches is not an option > with out changing the schema as the data nucleus does not persist row in > batches for tables which have foreign key on other tables. > > I see that best option is to persist the paths in parallel. It gave good > results. > > Solution Approach: > > I have used a thread pool to persist the snapshot. Size of this thread pool > is configurable. Paths for each object database/table are submitted to this > thread pool. If for reason some of the paths are not pesisted, snapshot is > removed and exception is throw back. > > This patch along with SENTRY-2423 was 5 times faster when tested with below. > > > > Object Type Count > Databases 209 > Tables 2100 > Partitions 200004 > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java > 092060c450c6a906850630cb10454737157af5fe > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > b387a22e40b8958395e1c12af12272b89a778726 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java > 0d62941a7bd45e0d8a67b5d95fdb39a8801f5a26 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 4a9afe303672baff39be01d4f190034b2bfb75fe > > > Diff: https://reviews.apache.org/r/68973/diff/5/ > > > Testing > ------- > > Added new test and also made sure that existing tests passed. > > > Thanks, > > kalyan kumar kalvagadda > >
