----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19703/#review38726 -----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/Constants.java <https://reviews.apache.org/r/19703/#comment71001> The list here is still mutable; "Changes to the returned list 'write through' to the array." (Java API) You can wrap the list using Collections.unmodifiableList to really lock it down. core/src/main/java/org/apache/accumulo/core/iterators/user/AgeOffFilter.java <https://reviews.apache.org/r/19703/#comment71002> I think threshold should be initialized to zero to mimic the prior behavior in the case where a user does not call init(). (Though they should.) The old code is strange in setting it to -1 before attempting to parse the TTL, almost as if -1 should be set if the parse throws an exception ... but that would stop the init() call. So perhaps -1 should properly be the default? Weird. server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java <https://reviews.apache.org/r/19703/#comment71003> Superficially this looks like a spot where UUID.randomUUID() would be helpful. Just something to consider, maybe for the future. trace/src/main/java/org/apache/accumulo/trace/instrument/TraceRunnable.java <https://reviews.apache.org/r/19703/#comment71005> Yeah, so: - Runnable doesn't implement Comparable, so the cast may fail. - The runnable field might be null. - Bill Havanki On March 26, 2014, 6:24 p.m., Mike Drob wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19703/ > ----------------------------------------------------------- > > (Updated March 26, 2014, 6:24 p.m.) > > > Review request for accumulo and Josh Elser. > > > Repository: accumulo > > > Description > ------- > > Address 'high' priority findbugs results. > > I ignored the majority of the default Charset warnings, but can go back and > fix them if folks decide that we should worry about them. > I ignored all of the name shadowing conflicts, because those only occured > when we moved classes between packages and had one extend the other for > backwards compatibility. Those should maybe go away in a version or two. > > I left several TODOs for places where I either couldn't figure out exactly > what to do, or figured that the work would be too extensive so I wanted some > community backing before proceeding. > > As the summary states, this is a Work In Progress. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/Constants.java > e0e88eb65c985123507d68cfd8d2440b4216648a > > core/src/main/java/org/apache/accumulo/core/client/admin/NamespaceOperationsImpl.java > 569a3b6b92d985e71cafdaee7b0cf6dbad6aa792 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java > c550f153cda75af328f829e287ad98509fed33d0 > core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java > 16f22e479188e81f1aea4a2f17a34b4d14d7c96e > > core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/ClassSize.java > 8abf425bdbdc4282255353304788f970a9597d09 > > core/src/main/java/org/apache/accumulo/core/iterators/user/AgeOffFilter.java > 6e9a571441b489670a93f7f6a3f4db06375bb782 > > core/src/main/java/org/apache/accumulo/core/util/shell/commands/ConfigCommand.java > c76a51fbca314a3fe0c8d3b0145eb54cd3a22030 > core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java > 2394063902c9dc86f56a1b184bb0d033ed857739 > > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java > 009988e355556d784fa9eaabd0846c0dd862cdab > > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java > 9aa2449dce2658f9d223b88143acd298a31df07e > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java > 2ef438ffdce65fe5504f40578a85a935564cfe3f > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java > f8b1702792bc8d05d94f57f1e071f386cc610fa4 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/OperationServlet.java > e40368779d0ca56ef780f1f7d2466da370b95595 > trace/src/main/java/org/apache/accumulo/trace/instrument/TraceRunnable.java > 41c765d253d8fc22e9ca62bd05c87882af20ab62 > > Diff: https://reviews.apache.org/r/19703/diff/ > > > Testing > ------- > > Unit tests, so far. > > > Thanks, > > Mike Drob > >
