accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 26530: ACCUMULO-1798 Add compaction strategy to user compactions
Date Thu, 16 Oct 2014 01:24:51 GMT


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


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