> On Oct. 13, 2014, 8:05 p.m., Josh Elser wrote: > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java, > > line 303 > > <https://reviews.apache.org/r/26530/diff/1/?file=717101#file717101line303> > > > > This condition strikes me a bit odd -- in the positive, we set config, > > but in the negative, we set iterators to null. A comment would help.
I think the intent of that code was to set this.iterators to null. But thats not needed, I thnik the else can just be removed. > On Oct. 13, 2014, 8:05 p.m., Josh Elser wrote: > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java, > > line 1781 > > <https://reviews.apache.org/r/26530/diff/1/?file=717108#file717108line1781> > > > > Obligatory TODO reminder (or remove the comment if thoroughly tested) oh yeah I tested that, thanks for the reminder. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26530/#review56431 ----------------------------------------------------------- On Oct. 9, 2014, 10:59 p.m., kturner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26530/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2014, 10:59 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-1798 > https://issues.apache.org/jira/browse/ACCUMULO-1798 > > > Repository: accumulo > > > Description > ------- > > This patch generated w/o thrift using the following command. > > git diff HEAD~1 -- core server shell test proxy/src/main/thrift/proxy.thrift > proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java > > The issue has the full patch. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/client/admin/CompactionConfig.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/CompactionStrategyConfig.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java > 97f538d > > core/src/main/java/org/apache/accumulo/core/client/impl/CompactionStrategyConfigUtil.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java > e46b9c9 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java > 08750fe > > core/src/test/java/org/apache/accumulo/core/client/impl/TableOperationsHelperTest.java > 02838ed > proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java a778add > proxy/src/main/thrift/proxy.thrift fbd9c52 > > server/base/src/main/java/org/apache/accumulo/server/master/tableOps/CompactionIterators.java > 4f5bf42 > > server/base/src/main/java/org/apache/accumulo/server/master/tableOps/UserCompactionConfig.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/FateServiceHandler.java > 41ca911 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/CompactRange.java > dedfb97 > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > 2e1eb2c > > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java > 4855a4f > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java > 6f69fb0 > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionStrategy.java > 7bc1a80 > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/DefaultCompactionStrategy.java > 8b03d17 > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/SizeLimitCompactionStrategy.java > 478939a > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java > 0194778 > shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java > 80dd9ba > test/src/test/java/org/apache/accumulo/proxy/SimpleProxyIT.java 6b4bcfb > test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 5a068af > test/src/test/java/org/apache/accumulo/test/UserCompactionStrategyIT.java > PRE-CREATION > > test/src/test/java/org/apache/accumulo/test/functional/FunctionalTestUtils.java > 1246efe > > Diff: https://reviews.apache.org/r/26530/diff/ > > > Testing > ------- > > mvn package so far > > > Thanks, > > kturner > >
