accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 24862: Accumulo-3021 Limit size of split rows
Date Thu, 21 Aug 2014 16:25:33 GMT

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


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