You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org> on 2018/10/19 18:13:42 UTC
Review Request 69087: SENTRY-2249: Enable batch insert of HMS paths
in Full Snapshot.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
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.
This patch will change with SENTRY-2305. This review request is just to give you a sence of the change.
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 b387a22e40b8958395e1c12af12272b89a778726
Diff: https://reviews.apache.org/r/69087/diff/1/
Testing
-------
Thanks,
kalyan kumar kalvagadda
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210254
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3304-3305 (original), 3311 (patched)
<https://reviews.apache.org/r/69087/#comment294885>
What changed here?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3327 (patched)
<https://reviews.apache.org/r/69087/#comment294886>
Why can't the MPath used here?
- Sergio Pena
On Oct. 31, 2018, 11:28 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, 11:28 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/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
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 66db6ae9a436b9728fb3c2ebdd21167ef042f937
>
>
> Diff: https://reviews.apache.org/r/69087/diff/3/
>
>
> 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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Nov. 1, 2018, 1:35 p.m., Sergio Pena wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Lines 37 (patched)
> > <https://reviews.apache.org/r/69087/diff/3/?file=2103484#file2103484line37>
> >
> > This is already defined in the package.jdo. Does it need to be defined here as well? My understanding is that it can be either on the package.jdo or the class file or both, but should we be consistent where we put these declarations?
That may be true as per documentation but I followed the convension used in sentry currently. We do mention it in both places. It should be of no harm.
> On Nov. 1, 2018, 1:35 p.m., Sergio Pena wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Line 36 (original), 42 (patched)
> > <https://reviews.apache.org/r/69087/diff/3/?file=2103484#file2103484line42>
> >
> > Is this variable needed now that you remove the getters and setters?
> >
> > Btw, why isn't this variable used anymore?
Collection of MPaths is initialized when snapshot is retrieved from database and is used with getPathStrings API.
> On Nov. 1, 2018, 1:35 p.m., Sergio Pena wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
> > Lines 259-261 (original), 259-261 (patched)
> > <https://reviews.apache.org/r/69087/diff/3/?file=2103487#file2103487line259>
> >
> > What is the strategy for the ID? Will it continue using the increment strategy?
It is not auto incremented. It is incremented in the application.
> On Nov. 1, 2018, 1:35 p.m., Sergio Pena wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
> > Lines 300 (patched)
> > <https://reviews.apache.org/r/69087/diff/3/?file=2103487#file2103487line300>
> >
> > Why a new class that points to the same table as MPath is needed?
This new class is added to make sure sure that batch insert can happen. With MPath, that can not be performed?
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210252
-----------------------------------------------------------
On Nov. 15, 2018, 9:02 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2018, 9:02 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/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/5/
>
>
> 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.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210252
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Lines 37 (patched)
<https://reviews.apache.org/r/69087/#comment294882>
This is already defined in the package.jdo. Does it need to be defined here as well? My understanding is that it can be either on the package.jdo or the class file or both, but should we be consistent where we put these declarations?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Line 36 (original), 42 (patched)
<https://reviews.apache.org/r/69087/#comment294883>
Is this variable needed now that you remove the getters and setters?
Btw, why isn't this variable used anymore?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 259-260 (original), 259-260 (patched)
<https://reviews.apache.org/r/69087/#comment294875>
There are weird characters at the end of these lines.
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 259-261 (original), 259-261 (patched)
<https://reviews.apache.org/r/69087/#comment294881>
What is the strategy for the ID? Will it continue using the increment strategy?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 300 (patched)
<https://reviews.apache.org/r/69087/#comment294879>
Why a new class that points to the same table as MPath is needed?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 301 (patched)
<https://reviews.apache.org/r/69087/#comment294880>
with our?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 303-311 (patched)
<https://reviews.apache.org/r/69087/#comment294877>
There are weird characters at the end of these lines.
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo
Lines 303-311 (patched)
<https://reviews.apache.org/r/69087/#comment294878>
There are weird characters at the end of these lines.
- Sergio Pena
On Oct. 31, 2018, 11:28 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, 11:28 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/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
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 66db6ae9a436b9728fb3c2ebdd21167ef042f937
>
>
> Diff: https://reviews.apache.org/r/69087/diff/3/
>
>
> 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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On Nov. 29, 2018, 6:44 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Lines 176 (patched)
> > <https://reviews.apache.org/r/69087/diff/10/?file=2111053#file2111053line182>
> >
> > do you want to clear pathsToPersist after persisting them? Otherwise, next time it is called, those paths will be added again
Persisting is the last thing that we do on an instance of MAuthzPathsMapping. This shouldn't be an issue.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210950
-----------------------------------------------------------
On Nov. 29, 2018, 6:32 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 6:32 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/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/10/
>
>
> 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.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Nov. 29, 2018, 6:44 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Lines 176 (patched)
> > <https://reviews.apache.org/r/69087/diff/10/?file=2111053#file2111053line182>
> >
> > do you want to clear pathsToPersist after persisting them? Otherwise, next time it is called, those paths will be added again
>
> kalyan kumar kalvagadda wrote:
> Persisting is the last thing that we do on an instance of MAuthzPathsMapping. This shouldn't be an issue.
what happens if makePersistent is called, then more paths are added, then makePersistent is called again? If you don't clean pathsToPersist, the old paths (already persisted) will be persisted again. It is a bug, and need to be fixed.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210950
-----------------------------------------------------------
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210950
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Lines 176 (patched)
<https://reviews.apache.org/r/69087/#comment295778>
do you want to clear pathsToPersist after persisting them? Otherwise, next time it is called, those paths will be added again
- Na Li
On Nov. 29, 2018, 6:32 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 6:32 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/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/10/
>
>
> 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.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211191
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211248
-----------------------------------------------------------
Fix it, then Ship it!
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Lines 192 (patched)
<https://reviews.apache.org/r/69087/#comment296181>
Kalyan this log message is wrong. Please change %d to {} or use String format
- Arjun Mishra
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211169
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211254
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3417 (original), 3433 (patched)
<https://reviews.apache.org/r/69087/#comment296185>
We need an else logic hear for case when MAuthzPathsMapping object already exists
for (String path: paths) {
mAuthzPathsMapping.addPathToPersist(path);
}
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3504 (original), 3509 (patched)
<https://reviews.apache.org/r/69087/#comment296186>
We are deleting objects but not the paths associated with them. We need to add "mAuthzPathsMapping.deletePersistent(pm, null);" before we delete the object
- Arjun Mishra
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211251
-----------------------------------------------------------
Kalyan,we need to persist MAuthzPathsMapping object at the end makepersistent. I tested and looks like MAuthzPathsMapping object that already exist are not getting updated
- Arjun Mishra
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211198
-----------------------------------------------------------
Ship it!
Ship It!
- Arjun Mishra
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review211255
-----------------------------------------------------------
Also test case to cover case when an object already exist and we add a path to it
- Arjun Mishra
On Nov. 29, 2018, 10:14 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69087/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2018, 10:14 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/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/11/
>
>
> 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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
(Updated Nov. 29, 2018, 10:14 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 (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/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/11/
Changes: https://reviews.apache.org/r/69087/diff/10-11/
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
(Updated Nov. 29, 2018, 6:32 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 (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/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/10/
Changes: https://reviews.apache.org/r/69087/diff/9-10/
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
(Updated Nov. 29, 2018, 6:29 p.m.)
Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
Changes
-------
addresed comment.
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/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/9/
Changes: https://reviews.apache.org/r/69087/diff/8-9/
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.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210949
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Line 78 (original), 88 (patched)
<https://reviews.apache.org/r/69087/#comment295777>
this seems to be a bug. If paths contains path_1, and caller adds path_2, this change results in paths contains only path_2, but lost path_1.
- Na Li
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210731
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3699 (original), 3700 (patched)
<https://reviews.apache.org/r/69087/#comment295463>
Could this way of allocating ID handle multiple threads without exception?
- 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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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)
> > <https://reviews.apache.org/r/69087/diff/7/?file=2108872#file2108872line45>
> >
> > 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.
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<MPath> 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)
> > <https://reviews.apache.org/r/69087/diff/7/?file=2108872#file2108872line107>
> >
> > 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.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210730
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3412 (original), 3430 (patched)
<https://reviews.apache.org/r/69087/#comment295462>
right now, only one thead from leader is calling this function, so assigning AuthzObjectID in this way is OK.
If in the future, multiple threads could call this function at the same time, will we have key conflict exception because multiple entries with same ID are inserted to DB?
- 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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210728
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 3457 (original), 3471 (patched)
<https://reviews.apache.org/r/69087/#comment295461>
why not call mAuthzPathsMapping.deletePersistent(pm, paths);?
So we only need to delete all paths with one call instead of loop through all paths and delete one path at one time.
- 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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On Nov. 20, 2018, 5:08 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
> > Lines 179 (patched)
> > <https://reviews.apache.org/r/69087/diff/7/?file=2108872#file2108872line187>
> >
> > can you set the filter that path can be one of the paths, so a single query can delete multiple paths?
You can use similar approach as QueryParamBuilder.addRolesFilter(), so a single query can delete multiple paths. And you can limit the number of filter entries to avoid too big query.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210722
-----------------------------------------------------------
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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210722
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Line 23 (original), 25 (patched)
<https://reviews.apache.org/r/69087/#comment295453>
why cannot we import specific classes?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Lines 40 (patched)
<https://reviews.apache.org/r/69087/#comment295454>
You already mentioned it is primary key in package.jdo line 259. I don't think you need to specify it here.
Can we just specify its meta data in package.jdo?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Line 109 (original), 119 (patched)
<https://reviews.apache.org/r/69087/#comment295455>
Can you handle the situation that mPaths is empty, but paths has content?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
Lines 179 (patched)
<https://reviews.apache.org/r/69087/#comment295457>
can you set the filter that path can be one of the paths, so a single query can delete multiple paths?
- 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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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 (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/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/6/
Changes: https://reviews.apache.org/r/69087/diff/5-6/
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
(Updated Nov. 15, 2018, 9:02 p.m.)
Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
Changes
-------
Made changes to isolate MPathToPersist to MAuthzPathsMapping so that MPathToPersist would be implementaion detail of MAuthzPathsMapping
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/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/5/
Changes: https://reviews.apache.org/r/69087/diff/4-5/
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/
-----------------------------------------------------------
(Updated Oct. 31, 2018, 11:28 p.m.)
Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
Changes
-------
updated new patch
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/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
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 66db6ae9a436b9728fb3c2ebdd21167ef042f937
Diff: https://reviews.apache.org/r/69087/diff/3/
Changes: https://reviews.apache.org/r/69087/diff/2-3/
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.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> 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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69087/#review210228
-----------------------------------------------------------
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/#comment294862>
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?
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/#comment294860>
can you use nextObjectId = getNextAuthzObjectID() and change line 3324 with "nextObjectId ++" to be consistent with line 3447?
- Na Li
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
>
>
Re: Review Request 69087: SENTRY-2249: Enable batch insert of HMS
paths in Full Snapshot.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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.
Changes
-------
Fixed some issue observed in mu testing.
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.
This patch will change with SENTRY-2305. This review request is just to give you a sence of the change.
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/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/
Changes: https://reviews.apache.org/r/69087/diff/1-2/
Testing
-------
Thanks,
kalyan kumar kalvagadda