----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69087/#review210726 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java Line 36 (original), 45 (patched) <https://reviews.apache.org/r/69087/#comment295458> Maintain both mPaths and paths in an instance of MAuthzPathsMapping is easy to have bug. Can you only have private Set<MPath> mPaths and not have paths? Then inside of this.makePersistent(), you create MPathToPersist from MPath value. That will minimize the changes to make, and avoid some scenarios you did not cover when having both mPaths and paths. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java Line 89 (original), 99 (patched) <https://reviews.apache.org/r/69087/#comment295459> since you have both mPaths and paths, if the call called addPath then getPath right away, the result is not correct. It is much easier to have just mPaths. - Na Li On Nov. 20, 2018, 12:22 a.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69087/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2018, 12:22 a.m.) > > > Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena. > > > Bugs: SENTRY-2249 > https://issues.apache.org/jira/browse/SENTRY-2249 > > > Repository: sentry > > > Description > ------- > > Currently each entry in full snapshot of HMS is persisted one entry at a > time. Instead it could be optimized by persisting the path entries in > batches. DB operations are expensive, reducing the number of database > operations and around trip time will help. This would decrease the time to > persist the snapshot in to database significantly. > > Size of the batch could be configurable. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java > d8c1061d36ed0b92116f0b2dd5ec820ccb166818 > > 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/model/MAuthzPathsMapping.java > c51f25a0393b482814afcd3b7a19e547b689ac6e > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MPath.java > b0eaff2120a80685da07c65a7706edf2be62ee01 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > e3ae24b0d11ec05537063e476a4a959bf2c43819 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > e2d6c85ac6b285e286c12f7eec669b841cf76e9d > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > ca8c41610d3dcd12b267579c3d3cbe5b7ba7b208 > > > Diff: https://reviews.apache.org/r/69087/diff/7/ > > > Testing > ------- > > Made sure all the unit tests passed. > > > Thanks, > > kalyan kumar kalvagadda > >
