> On Oct. 31, 2018, 4:33 p.m., Na Li wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > > Lines 303 (patched) > > <https://reviews.apache.org/r/69087/diff/2/?file=2103429#file2103429line303> > > > > why don't you replace MPath with MPathToPersist? > > > > In your current approach, two classes mapped into the same DB table and > > both use datastore to generate PATH_ID. Is it possible to have duplicate > > PATH_ID and cause persiting entry in AUTHZ_PATH to fail?
Purpose of MPathToPersist is dis-associate it self from MAuthzPathsMapping. We still need while retrieving the snapshot. We should use MPathToPersist for persisting and the MPath when lookup. I had a change making it clear. I missed uploading it. I will be updating it. There is not documentation that says we should not map multiple enties to same table. Let me give some insight to address your concern on duplicate PATH_ID. Each Entity that use sequence has an entry in SEQUENCE_TABLE to store the next sequence. In our case there will be seperate entry for org.apache.sentry.provider.db.service.model.MPath and org.apache.sentry.provider.db.service.model.MPathToPersist. If the application uses both MPath and MPathToPersist to persist data into AUTH_PATH there will be duplicates for PATH_ID. I will be making it clear in the comments. > On Oct. 31, 2018, 4:33 p.m., Na Li wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 3316 (patched) > > <https://reviews.apache.org/r/69087/diff/2/?file=2103430#file2103430line3317> > > > > can you use nextObjectId = getNextAuthzObjectID() and change line 3324 > > with "nextObjectId ++" to be consistent with line 3447? I did not use getNextAuthzObjectID deliberately for couple of reasons 1. it would add addtional lookup to the database for every object. 2. If we would like to go with multi-threaded approach. This does not work. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69087/#review210228 ----------------------------------------------------------- On Oct. 31, 2018, 12:58 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69087/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2018, 12:58 p.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/MPathToPersist.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > 20ec0deab6b97065cfe99beea3d14a6c7268aac3 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 33c40613a05f7c7fde314af6aba6b269bf6ffaae > > > Diff: https://reviews.apache.org/r/69087/diff/2/ > > > Testing > ------- > > Made sure all the unit tests passed. > > > Thanks, > > kalyan kumar kalvagadda > >
