You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Misha Dmitriev <mi...@cloudera.com> on 2017/08/22 23:02:30 UTC
Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/
-----------------------------------------------------------
Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
Bugs: SENTRY-1892
https://issues.apache.org/jira/browse/SENTRY-1892
Repository: sentry
Description
-------
SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
Diffs
-----
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
Diff: https://reviews.apache.org/r/61827/diff/1/
Testing
-------
Ran 'mvn install' locally, all tests passed.
Thanks,
Misha Dmitriev
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Misha Dmitriev <mi...@cloudera.com>.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > Overall looks good. This are partial comments - I still need more time to go through all the changes. These comments are all nits.
> >
> > Can you add a block comment at the top of HMSPaths.java explaining the trickery and the motivation for using trees rather then HashSet.
As we previously discussed in the e-mail: I myself am actually not 100% sure that we need TreeSets here. It would be good if a Sentry expert investigated this.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Line 166 (original), 166 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line166>
> >
> > Do we need a List here or we can use Collection? We just use it to walk entries.
Currently only Lists are used as arguments to this method, but in principle, having a Collection here gives us more flexibility. Changed.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Lines 234 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line234>
> >
> > Didn't you insert the element already on line 229 above?
No, in line 229 I created a TreeSet and initialized it with a single element (auth object) that was previosly referenced directly by the authzObjs field. And here I add a second element (passed to this method as an argument) to this set.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Line 502 (original), 530 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line534>
> >
> > This can be package-private
Done.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Lines 532 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line536>
> >
> > It may be easier to read (and make SonarLint happy) if you extract the ternary operation in a seperate statement. We don't need to save memory on Java lines :-)
Ok, replaced with if-else statements. Indeed this is not much longer and much easier to read.
> On Aug. 23, 2017, 12:50 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Lines 539 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line543>
> >
> > Same comment here about ternary op.
Changed.
- Misha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183559
-----------------------------------------------------------
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183559
-----------------------------------------------------------
Overall looks good. This are partial comments - I still need more time to go through all the changes. These comments are all nits.
Can you add a block comment at the top of HMSPaths.java explaining the trickery and the motivation for using trees rather then HashSet.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 166 (original), 166 (patched)
<https://reviews.apache.org/r/61827/#comment259592>
Do we need a List here or we can use Collection? We just use it to walk entries.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 234 (patched)
<https://reviews.apache.org/r/61827/#comment259603>
Didn't you insert the element already on line 229 above?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 502 (original), 530 (patched)
<https://reviews.apache.org/r/61827/#comment259598>
This can be package-private
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 532 (patched)
<https://reviews.apache.org/r/61827/#comment259601>
It may be easier to read (and make SonarLint happy) if you extract the ternary operation in a seperate statement. We don't need to save memory on Java lines :-)
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 537 (patched)
<https://reviews.apache.org/r/61827/#comment259599>
This can be package-private
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 539 (patched)
<https://reviews.apache.org/r/61827/#comment259602>
Same comment here about ternary op.
- Alexander Kolbasov
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On Aug. 23, 2017, 6:35 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Lines 537 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line541>
> >
> > Looks like most callers just care whether size() != 0, so you may have even simpler isEmpty() function.
>
> Misha Dmitriev wrote:
> In principle, yes, but let's have some more flexibility for the future. Looks like size == 1 is a very frequent case and maybe the rest of the Sentry code will benefit from easily accessible knowledge about it.
Can we have two functions - size() and isEmpty()?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183637
-----------------------------------------------------------
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Misha Dmitriev <mi...@cloudera.com>.
> On Aug. 23, 2017, 6:35 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
> > Lines 537 (patched)
> > <https://reviews.apache.org/r/61827/diff/1/?file=1801885#file1801885line541>
> >
> > Looks like most callers just care whether size() != 0, so you may have even simpler isEmpty() function.
In principle, yes, but let's have some more flexibility for the future. Looks like size == 1 is a very frequent case and maybe the rest of the Sentry code will benefit from easily accessible knowledge about it.
- Misha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183637
-----------------------------------------------------------
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183637
-----------------------------------------------------------
This is the last comment I have - looks good!
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 537 (patched)
<https://reviews.apache.org/r/61827/#comment259698>
Looks like most callers just care whether size() != 0, so you may have even simpler isEmpty() function.
- Alexander Kolbasov
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Vadim Spector <vs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183680
-----------------------------------------------------------
Ship it!
Ship It!
- Vadim Spector
On Aug. 23, 2017, 10:08 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2017, 10:08 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/3/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/
-----------------------------------------------------------
(Updated Aug. 23, 2017, 10:08 p.m.)
Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
Changes
-------
Addressed Vadim's comment about logging.
Bugs: SENTRY-1892
https://issues.apache.org/jira/browse/SENTRY-1892
Repository: sentry
Description
-------
SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
Diff: https://reviews.apache.org/r/61827/diff/3/
Changes: https://reviews.apache.org/r/61827/diff/2-3/
Testing
-------
Ran 'mvn install' locally, all tests passed.
Thanks,
Misha Dmitriev
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183649
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On Aug. 23, 2017, 7:29 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2017, 7:29 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/2/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Vadim Spector <vs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183647
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java
Line 52 (original), 53 (patched)
<https://reviews.apache.org/r/61827/#comment259714>
This method is only called to get a full image, which is a special event, certainly not something happening routinely. It would be very useful if this method logged at INFO level the dump creation event, plus some stats like # of redundant strings, which could be supplied by DupDetector.
- Vadim Spector
On Aug. 23, 2017, 7:29 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2017, 7:29 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/2/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/
-----------------------------------------------------------
(Updated Aug. 23, 2017, 7:29 p.m.)
Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
Changes
-------
Addressed the comments.
Bugs: SENTRY-1892
https://issues.apache.org/jira/browse/SENTRY-1892
Repository: sentry
Description
-------
SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
Diff: https://reviews.apache.org/r/61827/diff/2/
Changes: https://reviews.apache.org/r/61827/diff/1-2/
Testing
-------
Ran 'mvn install' locally, all tests passed.
Thanks,
Misha Dmitriev
Re: Review Request 61827: SENTRY-1892: Reduce memory consumption of
HMSPath and TPathEntry
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61827/#review183646
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 537 (patched)
<https://reviews.apache.org/r/61827/#comment259713>
NP, added.
- Misha Dmitriev
On Aug. 22, 2017, 11:02 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61827/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2017, 11:02 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, and Vadim Spector.
>
>
> Bugs: SENTRY-1892
> https://issues.apache.org/jira/browse/SENTRY-1892
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1892: Reduce memory consumption of HMSPath and TPathEntry
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathEntry.java 2a89368208bcef5b537cb0c2d59fd14b1735f435
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 6e71561e36d7f235afb14961299bfc23c03607a6
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java bf6c3dec35c7d22b9bd0e3f863925f7ff3e94515
> sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift 22dea02ea003530513d7fb270eb0ca0ce38957e9
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java a0d7bdcd256a17558178ebc5238b1f3da9fa5e9b
>
>
> Diff: https://reviews.apache.org/r/61827/diff/1/
>
>
> Testing
> -------
>
> Ran 'mvn install' locally, all tests passed.
>
>
> Thanks,
>
> Misha Dmitriev
>
>