----------------------------------------------------------- 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 > >
