accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 29176: ACCUMULO-3420 Get Visibility Metrics from PrintInfo
Date Thu, 18 Dec 2014 16:19:56 GMT

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


A unit test would be nice. There's a few dangling whitespace at end of line (one in javadoc
and a few on empty lines) that would be nice to clean up too.


core/src/main/java/org/apache/accumulo/core/file/rfile/MetricsGatherer.java
<https://reviews.apache.org/r/29176/#comment108706>

    RFileVisibilityMetrics might be a better name. This class isn't actually doing any gathering,
the caller is.



core/src/main/java/org/apache/accumulo/core/file/rfile/MetricsGatherer.java
<https://reviews.apache.org/r/29176/#comment108707>

    Why the concurrent maps? I don't see anything in these changes that would require it now,
do you have plans for something else in the future?



core/src/main/java/org/apache/accumulo/core/file/rfile/MetricsGatherer.java
<https://reviews.apache.org/r/29176/#comment108708>

    It would be nicer to use a logger instead of System.out, but I can understand the intent
of what you did since PrintInfo works that way too. Just a comment.



core/src/main/java/org/apache/accumulo/core/file/rfile/MetricsGatherer.java
<https://reviews.apache.org/r/29176/#comment108709>

    What does hashing the visibility label get you? If you can invoke this program on some
file, your data is already insecure (by virtue of it being accessible outside of Accumulo).


- Josh Elser


On Dec. 17, 2014, 8:31 p.m., Jenna Huston wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29176/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2014, 8:31 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3420
>     https://issues.apache.org/jira/browse/ACCUMULO-3420
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Added an option to PrintInfo to get visibility metrics.  Prints the number of times a
visibilty is seen in each locality group.  Also shows how many blocks in the locality group
have his visibiltiy.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/file/rfile/MetricsGatherer.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java 43586dd 
>   core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 9dcb3a5 
> 
> Diff: https://reviews.apache.org/r/29176/diff/
> 
> 
> Testing
> -------
> 
> Tested with a few RFiles that I made.
> 
> 
> Thanks,
> 
> Jenna Huston
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message