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

Ship it!


Unit tests are very well written (comments are very nice to see). In the 
future, it would be nice to see some more comments on the changes in the "real" 
code (referencing the JIRA issue with a short description is always nice to 
see). Overall, looks good!


test/src/test/java/org/apache/accumulo/test/LargeSplitRowIT.java
<https://reviews.apache.org/r/24862/#comment89202>

    Minor complaint: it's nice to always include a message as to why the test 
fails, but your comment describes this well.


- Josh Elser


On Aug. 20, 2014, 7:45 p.m., Jenna Huston wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24862/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2014, 7:45 p.m.)
> 
> 
> Review request for accumulo and kturner.
> 
> 
> Bugs: Accumulo-3021
>     https://issues.apache.org/jira/browse/Accumulo-3021
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Added constraint for maximum end row size. Checked this constraint before any 
> split is done.
> A unit test was added to test the added constraint when both a user initiaed 
> a tablet split and when an automatic split occurs.  This test also tests to 
> make sure that when new data is added to a table that is small enough to be 
> used as an end row the table splits.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 85c56f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 
> 82045b6 
>   test/src/test/java/org/apache/accumulo/test/LargeSplitRowIT.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24862/diff/
> 
> 
> Testing
> -------
> 
> Passes unit tests.
> 
> 
> Thanks,
> 
> Jenna Huston
> 
>

Reply via email to