accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 28739: ACCUMULO-3134 Added file selection and output configuration to compact command
Date Fri, 05 Dec 2014 18:17:45 GMT


> On Dec. 5, 2014, 3:45 p.m., Josh Elser wrote:
> > Lots of whitespace nits to resolve (pretty much every file). Need tests (if not
via the Shell, at least some sort of mock) for all of the options.

I'll wack the whitespace


> On Dec. 5, 2014, 3:45 p.m., Josh Elser wrote:
> > shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java, line
225
> > <https://reviews.apache.org/r/28739/diff/1/?file=783471#file783471line225>
> >
> >     What's the intent behind this option? Why would you ever want to compact some
of the files of a table with a different compression codec than what the table default is?
> >     
> >     Only reason I can come up with is importing files written with a different codec
and wanting to compact just those files.
> >     
> >     Actually, I'd have the same question against, datablock size, indexblock size,
hdfs replication, and hdfs block size options too.

One use case I was thinking of is using the compaction range option along w/ these options
to compact one tablet differently for performance testing.


> On Dec. 5, 2014, 3:45 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/compaction/CompactionSettings.java,
line 40
> > <https://reviews.apache.org/r/28739/diff/1/?file=783468#file783468line40>
> >
> >     Use Long.toString(long) and Integer.toString(int), respectively.

good point, I suppose that would be more type safe.


> On Dec. 5, 2014, 3:45 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/strategies/ConfigurableCompactionStrategy.java,
line 145
> > <https://reviews.apache.org/r/28739/diff/1/?file=783469#file783469line145>
> >
> >     Reminder, you can start using java7 constructs.

I have been inconsistently doing that.


- kturner


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


On Dec. 5, 2014, 4:10 a.m., kturner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28739/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2014, 4:10 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3134
>     https://issues.apache.org/jira/browse/ACCUMULO-3134
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Added file selection and output configuration to shell compact command
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/compaction/CompactionSettings.java PRE-CREATION

>   server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/strategies/ConfigurableCompactionStrategy.java
PRE-CREATION 
>   shell/src/main/java/org/apache/accumulo/shell/Shell.java 8927ee0 
>   shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java 660630e

>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java d878c7f 
> 
> Diff: https://reviews.apache.org/r/28739/diff/
> 
> 
> Testing
> -------
> 
> Added an IT that test file selection options on compact command.  IT does not test output
file configuration.
> 
> 
> Thanks,
> 
> kturner
> 
>


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