Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS paths in Full Snapshot.

[kalyan kumar kalvagadda via Review Board]

> On Nov. 20, 2018, 5:20 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Line 36 (original), 45 (patched)
> > 
> >
> > Maintain both mPaths and paths in an instance of MAuthzPathsMapping is 
> > easy to have bug.
> > 
> > Can you only have private Set 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.

Doing that is not efficient because we will be converting strings -> MPath -> 
MPathToPesist. That is why i took this route. Let me think.
More over using collection to hold that should be persisted would make 
the code much complex. I have already tried it. It will lead to bugs in the 
future


> On Nov. 20, 2018, 5:20 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Line 89 (original), 99 (patched)
> > 
> >
> > 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.

It may look easier to have just mPaths but it is not. I have spent hours to 
make it work. It was becoming more complex.


- kalyan kumar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210726
---


On Nov. 21, 2018, 7:48 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> ---
> 
> (Updated Nov. 21, 2018, 7:48 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/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/NotificationProcessor.java
>  ab262d0a849852bd95d88dd099dc6bf187715cad 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
>  f5802d70145474c173fd4abfd2d2189e729e170c 
>   
> 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/8/
> 
> 
> Testing
> ---
> 
> Made sure all the unit tests passed.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS paths in Full Snapshot.

[kalyan kumar kalvagadda via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
---

(Updated Nov. 21, 2018, 7:48 p.m.)


Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.


Changes
---

Addressed reiview comments from lina.


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 (updated)
-

  
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/NotificationProcessor.java
 ab262d0a849852bd95d88dd099dc6bf187715cad 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java
 f5802d70145474c173fd4abfd2d2189e729e170c 
  
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/8/

Changes: https://reviews.apache.org/r/69087/diff/7-8/


Testing
---

Made sure all the unit tests passed.


Thanks,

kalyan kumar kalvagadda