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/06/22 18:45:04 UTC
Review Request 60373: SENTRY-1811: reduce memory consumed by data
structures used in HDFS sync
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60373/
-----------------------------------------------------------
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Bugs: SENTRY-1811
https://issues.apache.org/jira/browse/SENTRY-1811
Repository: sentry
Description
-------
SENTRY-1811: reduce memory consumed by data structures used in HDFS sync
Diffs
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 3fb9b4e3829700ba8df7ca1a6f21b05185c33e6b
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java 4e736ea45a763d0af846be2aa8773feb4ec1d1df
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java f31ec215d20c5898976148c627e81e7d26151655
Diff: https://reviews.apache.org/r/60373/diff/1/
Testing
-------
Thanks,
Misha Dmitriev
Re: Review Request 60373: SENTRY-1811: reduce memory consumed by data
structures used in HDFS sync
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60373/
-----------------------------------------------------------
(Updated June 23, 2017, 7:16 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Addressed the comments.
Bugs: SENTRY-1811
https://issues.apache.org/jira/browse/SENTRY-1811
Repository: sentry
Description
-------
SENTRY-1811: reduce memory consumed by data structures used in HDFS sync
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 18c921da15d66ea3586ad1eecab8f551521eaf57
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java 7dfb5fa078ce8bba213b0f50d64af20f45039f82
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java f31ec215d20c5898976148c627e81e7d26151655
Diff: https://reviews.apache.org/r/60373/diff/3/
Changes: https://reviews.apache.org/r/60373/diff/2-3/
Testing
-------
Thanks,
Misha Dmitriev
Re: Review Request 60373: SENTRY-1811: reduce memory consumed by data
structures used in HDFS sync
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60373/#review178806
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 193 (patched)
<https://reviews.apache.org/r/60373/#comment253032>
Done.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 198 (patched)
<https://reviews.apache.org/r/60373/#comment253033>
Done.
- Misha Dmitriev
On June 22, 2017, 11:29 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60373/
> -----------------------------------------------------------
>
> (Updated June 22, 2017, 11:29 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1811
> https://issues.apache.org/jira/browse/SENTRY-1811
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1811: reduce memory consumed by data structures used in HDFS sync
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 3fb9b4e3829700ba8df7ca1a6f21b05185c33e6b
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java 4e736ea45a763d0af846be2aa8773feb4ec1d1df
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java f31ec215d20c5898976148c627e81e7d26151655
>
>
> Diff: https://reviews.apache.org/r/60373/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 60373: SENTRY-1811: reduce memory consumed by data
structures used in HDFS sync
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60373/#review178799
-----------------------------------------------------------
Fix it, then Ship it!
The original patch is for sentry-ha-redesign branch, not master. Changes look good, some nits below.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 193 (patched)
<https://reviews.apache.org/r/60373/#comment253007>
Naming - this is predicate, so may be use something like hasChildren()?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Lines 198 (patched)
<https://reviews.apache.org/r/60373/#comment253006>
I think the official way is to return Collections.emptyList()
why not use
return (children != null) ? children.values() : COllections.emptySet();
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java
Line 462 (original), 504 (patched)
<https://reviews.apache.org/r/60373/#comment253008>
Use Collections.emptySet()
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java
Lines 73 (patched)
<https://reviews.apache.org/r/60373/#comment253009>
Collections.emptySet()
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java
Line 73 (original), 76 (patched)
<https://reviews.apache.org/r/60373/#comment253010>
Use isEmpty() instead of size() != 0
- Alexander Kolbasov
On June 22, 2017, 11:29 p.m., Misha Dmitriev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60373/
> -----------------------------------------------------------
>
> (Updated June 22, 2017, 11:29 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1811
> https://issues.apache.org/jira/browse/SENTRY-1811
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1811: reduce memory consumed by data structures used in HDFS sync
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 3fb9b4e3829700ba8df7ca1a6f21b05185c33e6b
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java 4e736ea45a763d0af846be2aa8773feb4ec1d1df
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java f31ec215d20c5898976148c627e81e7d26151655
>
>
> Diff: https://reviews.apache.org/r/60373/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Misha Dmitriev
>
>
Re: Review Request 60373: SENTRY-1811: reduce memory consumed by data
structures used in HDFS sync
Posted by Misha Dmitriev <mi...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60373/
-----------------------------------------------------------
(Updated June 22, 2017, 11:29 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Added several more String.intern() calls based on string duplication stats in other Sentry heap dumps.
Bugs: SENTRY-1811
https://issues.apache.org/jira/browse/SENTRY-1811
Repository: sentry
Description
-------
SENTRY-1811: reduce memory consumed by data structures used in HDFS sync
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java 3fb9b4e3829700ba8df7ca1a6f21b05185c33e6b
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java 4e736ea45a763d0af846be2aa8773feb4ec1d1df
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPaths.java f31ec215d20c5898976148c627e81e7d26151655
Diff: https://reviews.apache.org/r/60373/diff/2/
Changes: https://reviews.apache.org/r/60373/diff/1-2/
Testing
-------
Thanks,
Misha Dmitriev