You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Sean Busbey <se...@manvsbeard.com> on 2013/11/14 17:29:29 UTC
Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter might
not write the specified number of rowids
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-1892
https://issues.apache.org/jira/browse/ACCUMULO-1892
Repository: accumulo
Description
-------
ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
Diffs
-----
src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
Diff: https://reviews.apache.org/r/15517/diff/
Testing
-------
unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
Thanks,
Sean Busbey
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/#review28969
-----------------------------------------------------------
Ship it!
(non-binding)
- Bill Havanki
On Nov. 14, 2013, 5:23 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15517/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2013, 5:23 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1892
> https://issues.apache.org/jira/browse/ACCUMULO-1892
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
>
>
> Diffs
> -----
>
> src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
>
> Diff: https://reviews.apache.org/r/15517/diff/
>
>
> Testing
> -------
>
> unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Nov. 15, 2013, 6:03 p.m., kturner wrote:
> > src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java, line 133
> > <https://reviews.apache.org/r/15517/diff/2/?file=384959#file384959line133>
> >
> > Not sure if "atleast" is a word. I did not notice this in the code review, but when I ran the program and triggered this case it really stuck out. I can fix this after applying the patch unless you would like to fix it.
I'll fix it in the patch submitted on the ticket.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/#review28978
-----------------------------------------------------------
On Nov. 14, 2013, 10:23 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15517/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2013, 10:23 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1892
> https://issues.apache.org/jira/browse/ACCUMULO-1892
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
>
>
> Diffs
> -----
>
> src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
>
> Diff: https://reviews.apache.org/r/15517/diff/
>
>
> Testing
> -------
>
> unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/#review28978
-----------------------------------------------------------
Ship it!
The changes look good. I also pulled down the patch and tried running it, works great.
src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java
<https://reviews.apache.org/r/15517/#comment56032>
Not sure if "atleast" is a word. I did not notice this in the code review, but when I ran the program and triggered this case it really stuck out. I can fix this after applying the patch unless you would like to fix it.
- kturner
On Nov. 14, 2013, 10:23 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15517/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2013, 10:23 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1892
> https://issues.apache.org/jira/browse/ACCUMULO-1892
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
>
>
> Diffs
> -----
>
> src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
>
> Diff: https://reviews.apache.org/r/15517/diff/
>
>
> Testing
> -------
>
> unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/
-----------------------------------------------------------
(Updated Nov. 14, 2013, 10:23 p.m.)
Review request for accumulo.
Changes
-------
Updated based on feedback from Bill. Now properly sets exit error status on all "something went wrong" code paths.
Bugs: ACCUMULO-1892
https://issues.apache.org/jira/browse/ACCUMULO-1892
Repository: accumulo
Description
-------
ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
Diffs (updated)
-----
src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
Diff: https://reviews.apache.org/r/15517/diff/
Testing
-------
unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
Thanks,
Sean Busbey
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Nov. 14, 2013, 10:03 p.m., Bill Havanki wrote:
> > src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java, line 156
> > <https://reviews.apache.org/r/15517/diff/1/?file=384504#file384504line156>
> >
> > I believe this will lead to an infinite loop if max - min < num. The loop will keep adding numbers in the range [min, max) but cannot terminate because the set cannot grow beyond size max - min.
> >
> > I think the check for this condition above should do System.exit(1).
yep! exactly why the above check was added. :/
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/#review28912
-----------------------------------------------------------
On Nov. 14, 2013, 4:29 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15517/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2013, 4:29 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1892
> https://issues.apache.org/jira/browse/ACCUMULO-1892
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
>
>
> Diffs
> -----
>
> src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
>
> Diff: https://reviews.apache.org/r/15517/diff/
>
>
> Testing
> -------
>
> unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 15517: ACCUMULO-1892 examples.simple.RandomBatchWriter
might not write the specified number of rowids
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15517/#review28912
-----------------------------------------------------------
src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java
<https://reviews.apache.org/r/15517/#comment55978>
I believe this will lead to an infinite loop if max - min < num. The loop will keep adding numbers in the range [min, max) but cannot terminate because the set cannot grow beyond size max - min.
I think the check for this condition above should do System.exit(1).
- Bill Havanki
On Nov. 14, 2013, 11:29 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15517/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2013, 11:29 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1892
> https://issues.apache.org/jira/browse/ACCUMULO-1892
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-1892 changes RandomBatchWriter to ensure it writes the specified number of rowids.
>
>
> Diffs
> -----
>
> src/examples/simple/src/main/java/org/apache/accumulo/examples/simple/client/RandomBatchWriter.java 3206fa6bbb5e560175e9aec6a1e612185e4ca945
>
> Diff: https://reviews.apache.org/r/15517/diff/
>
>
> Testing
> -------
>
> unit tests pass. simple.examples.Examples functional tests pass (given fixes in ACCUMULO-1878 to actually run them).
>
>
> Thanks,
>
> Sean Busbey
>
>