You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Mike Drob <md...@mdrob.com> on 2014/02/25 00:35:49 UTC
Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2399
https://issues.apache.org/jira/browse/ACCUMULO-2399
Repository: accumulo
Description
-------
ACCUMULO-2399 Continuous* wait a bit for scanners
When starting ContinuousIngest concurrently with consumers, many of the
consumers would die if they came up before the table had been created.
Make the code a bit more robust during start-up.
Diffs
-----
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
Diff: https://reviews.apache.org/r/18444/diff/
Testing
-------
Unit tests. Started processes on 3 node cluster and batch walkers did not die.
Thanks,
Mike Drob
Re: Review Request 18444: ACCUMULO-2399 Alert user CI table should exist
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On Feb. 25, 2014, 4:22 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java, lines 153-155
> > <https://reviews.apache.org/r/18444/diff/2/?file=503673#file503673line153>
> >
> > Can we include the table name in the error message? Is it worth logging at FATAL?
+1
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35464
-----------------------------------------------------------
On Feb. 25, 2014, 3:46 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 3:46 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Alert user CI table should exist
>
> Extract common table checking functionality for continuous clients and
> fail fast when the table does not exist. No longer create the table in
> ingest to make the requirement explicit.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java b63efbc9c7423be8985a292d0c162a672e4b65a0
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started continuous ingest against a local cluster and it did not run.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Alert user CI table should exist
Posted by Mike Drob <md...@mdrob.com>.
> On Feb. 25, 2014, 9:22 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java, lines 153-155
> > <https://reviews.apache.org/r/18444/diff/2/?file=503673#file503673line153>
> >
> > Can we include the table name in the error message? Is it worth logging at FATAL?
>
> Bill Havanki wrote:
> +1
The table name is automatically included in the exception message as part of the TNFE constructor. When accessing "tableName", this trace will look something like:
Exception in thread "main" org.apache.accumulo.core.client.TableNotFoundException: Table tableName does not exist (Consult the README and create the table before starting test processes.)
at org.apache.accumulo.server.test.continuous.ContinuousUtil.createScanner(ContinuousUtil.java:50)
Also, I'd rather not log-and-throw. Rationale: https://today.java.net/article/2006/04/04/exception-handling-antipatterns#logAndThrow
> On Feb. 25, 2014, 9:22 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java, line 32
> > <https://reviews.apache.org/r/18444/diff/2/?file=503676#file503676line32>
> >
> > nit: trailing whitespace
Will fix before committing.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35464
-----------------------------------------------------------
On Feb. 25, 2014, 8:46 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 8:46 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Alert user CI table should exist
>
> Extract common table checking functionality for continuous clients and
> fail fast when the table does not exist. No longer create the table in
> ingest to make the requirement explicit.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java b63efbc9c7423be8985a292d0c162a672e4b65a0
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started continuous ingest against a local cluster and it did not run.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Alert user CI table should exist
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35464
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java
<https://reviews.apache.org/r/18444/#comment65995>
Can we include the table name in the error message? Is it worth logging at FATAL?
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java
<https://reviews.apache.org/r/18444/#comment65996>
nit: trailing whitespace
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java
<https://reviews.apache.org/r/18444/#comment65997>
similar to comment in the ingest code: could we include the table name and is it worth logging at FATAL?
- Sean Busbey
On Feb. 25, 2014, 8:46 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 8:46 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Alert user CI table should exist
>
> Extract common table checking functionality for continuous clients and
> fail fast when the table does not exist. No longer create the table in
> ingest to make the requirement explicit.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java b63efbc9c7423be8985a292d0c162a672e4b65a0
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started continuous ingest against a local cluster and it did not run.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Alert user CI table should exist
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35467
-----------------------------------------------------------
Aside: it's also probably worth verifying that when you do create the table ahead of time the test runs.
- Sean Busbey
On Feb. 25, 2014, 8:46 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2014, 8:46 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Alert user CI table should exist
>
> Extract common table checking functionality for continuous clients and
> fail fast when the table does not exist. No longer create the table in
> ingest to make the requirement explicit.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java b63efbc9c7423be8985a292d0c162a672e4b65a0
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started continuous ingest against a local cluster and it did not run.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Alert user CI table should exist
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/
-----------------------------------------------------------
(Updated Feb. 25, 2014, 8:46 p.m.)
Review request for accumulo.
Changes
-------
Instead of sleeping, have the continuous clients output a slightly more informative error message. Remove the table creation code path from continuous ingest.
Summary (updated)
-----------------
ACCUMULO-2399 Alert user CI table should exist
Bugs: ACCUMULO-2399
https://issues.apache.org/jira/browse/ACCUMULO-2399
Repository: accumulo
Description (updated)
-------
ACCUMULO-2399 Alert user CI table should exist
Extract common table checking functionality for continuous clients and
fail fast when the table does not exist. No longer create the table in
ingest to make the requirement explicit.
Diffs (updated)
-----
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousIngest.java b63efbc9c7423be8985a292d0c162a672e4b65a0
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
Diff: https://reviews.apache.org/r/18444/diff/
Testing (updated)
-------
Unit tests. Started continuous ingest against a local cluster and it did not run.
Thanks,
Mike Drob
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Feb. 25, 2014, 12:38 a.m., Josh Elser wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java, line 46
> > <https://reviews.apache.org/r/18444/diff/1/?file=502880#file502880line46>
> >
> > Might as well just sleep/check/repeat indefinitely. The stop script just pkill's the process.
>
> Sean Busbey wrote:
> That will further hide the error though. Especially for tests that run for 3 days, I want failures to happen up front and loudly. Because, by necessity, most people are going fire and then check back on logs and such after the elapsed time is up. Schedules get real hard when you sink a whole 3 day turn around.
>
> Josh Elser wrote:
> I would've assumed that you check to make sure things are running before you ignore it for a long period of time, but that's just me :P. If we're going to be touching this code, just thinking about making something more robust than a "wait 3 seconds" because I know my attention span and it can certainly exceed that which makes it no better than not waiting.
Depends on how long you wait though. What if the table creation is blocked waiting on some FATE operation for 30 seconds? 5 minutes? It's just better to make things have easily observable state. Creating the table ahead of time in the shell makes it very obvious if things are correct (or if you are blocked waiting).
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35356
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Feb. 25, 2014, 12:38 a.m., Josh Elser wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java, line 46
> > <https://reviews.apache.org/r/18444/diff/1/?file=502880#file502880line46>
> >
> > Might as well just sleep/check/repeat indefinitely. The stop script just pkill's the process.
That will further hide the error though. Especially for tests that run for 3 days, I want failures to happen up front and loudly. Because, by necessity, most people are going fire and then check back on logs and such after the elapsed time is up. Schedules get real hard when you sink a whole 3 day turn around.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35356
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Josh Elser <jo...@gmail.com>.
> On Feb. 25, 2014, 12:38 a.m., Josh Elser wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java, line 46
> > <https://reviews.apache.org/r/18444/diff/1/?file=502880#file502880line46>
> >
> > Might as well just sleep/check/repeat indefinitely. The stop script just pkill's the process.
>
> Sean Busbey wrote:
> That will further hide the error though. Especially for tests that run for 3 days, I want failures to happen up front and loudly. Because, by necessity, most people are going fire and then check back on logs and such after the elapsed time is up. Schedules get real hard when you sink a whole 3 day turn around.
I would've assumed that you check to make sure things are running before you ignore it for a long period of time, but that's just me :P. If we're going to be touching this code, just thinking about making something more robust than a "wait 3 seconds" because I know my attention span and it can certainly exceed that which makes it no better than not waiting.
- Josh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35356
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35356
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java
<https://reviews.apache.org/r/18444/#comment65822>
Might as well just sleep/check/repeat indefinitely. The stop script just pkill's the process.
- Josh Elser
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Mike Drob <md...@mdrob.com>.
> On Feb. 25, 2014, 12:23 a.m., Sean Busbey wrote:
> > Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
> >
> > As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
>
> Mike Drob wrote:
> The README already instructs the user to create the table before running. It could be made more clear, though.
>
> Sean Busbey wrote:
> If the README already tells the user to create the table all the more reason to enforce following the instructions.
So you suggest removing the table creation code path from ingest? I'd be ok with that.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Feb. 25, 2014, 12:23 a.m., Sean Busbey wrote:
> > Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
> >
> > As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
>
> Mike Drob wrote:
> The README already instructs the user to create the table before running. It could be made more clear, though.
If the README already tells the user to create the table all the more reason to enforce following the instructions.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On Feb. 24, 2014, 7:23 p.m., Sean Busbey wrote:
> > Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
> >
> > As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
>
> Mike Drob wrote:
> The README already instructs the user to create the table before running. It could be made more clear, though.
>
> Sean Busbey wrote:
> If the README already tells the user to create the table all the more reason to enforce following the instructions.
>
> Mike Drob wrote:
> So you suggest removing the table creation code path from ingest? I'd be ok with that.
>
> Sean Busbey wrote:
> yeah. maybe update the error handling in both ingest and the reader clients so that when the table doesn't exists it says to go back to the instructions on creating the table.
+1
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
On Feb. 24, 2014, 6:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Mike Drob <md...@mdrob.com>.
> On Feb. 25, 2014, 12:23 a.m., Sean Busbey wrote:
> > Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
> >
> > As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
The README already instructs the user to create the table before running. It could be made more clear, though.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Feb. 25, 2014, 12:23 a.m., Sean Busbey wrote:
> > Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
> >
> > As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
>
> Mike Drob wrote:
> The README already instructs the user to create the table before running. It could be made more clear, though.
>
> Sean Busbey wrote:
> If the README already tells the user to create the table all the more reason to enforce following the instructions.
>
> Mike Drob wrote:
> So you suggest removing the table creation code path from ingest? I'd be ok with that.
yeah. maybe update the error handling in both ingest and the reader clients so that when the table doesn't exists it says to go back to the instructions on creating the table.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35352
-----------------------------------------------------------
Is there a disadvantage to instead updating the README to require creating the table ahead of time? That way we could avoid this code, and we could remove the table creation code from the writers.
As a side effect, there'd also be a set where the tester would implicitly confirm that the user/password for accessing Accumulo works and can deal with the table.
- Sean Busbey
On Feb. 24, 2014, 11:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 11:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Bill Havanki <bh...@clouderagovt.com>.
> On Feb. 25, 2014, 9:01 a.m., Bill Havanki wrote:
> > src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java, line 46
> > <https://reviews.apache.org/r/18444/diff/1/?file=502880#file502880line46>
> >
> > What if, due to the cluster environment, it always takes longer than 3 seconds? Should the interval be configurable?
> >
> > How about requiring the table to be manually created first? Then this just fails quick with a nice error "make the table and try again".
Sorry, this somehow got logged outside the correct thread.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35398
-----------------------------------------------------------
On Feb. 24, 2014, 6:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 18444: ACCUMULO-2399 Continuous* wait a bit for scanners
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18444/#review35398
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java
<https://reviews.apache.org/r/18444/#comment65889>
What if, due to the cluster environment, it always takes longer than 3 seconds? Should the interval be configurable?
How about requiring the table to be manually created first? Then this just fails quick with a nice error "make the table and try again".
src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java
<https://reviews.apache.org/r/18444/#comment65888>
You might as well just let the InterruptedException propagate out. An interrupt during the sleep should be interpreted as a signal to cancel the test.
- Bill Havanki
On Feb. 24, 2014, 6:35 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18444/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:35 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2399
> https://issues.apache.org/jira/browse/ACCUMULO-2399
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2399 Continuous* wait a bit for scanners
>
> When starting ContinuousIngest concurrently with consumers, many of the
> consumers would die if they came up before the table had been created.
> Make the code a bit more robust during start-up.
>
>
> Diffs
> -----
>
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousBatchWalker.java 4659affc49fc7fc2519e092330dda817ccbbadcd
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousQuery.java c8ae6ecf9e5ef2bd51cb803cba429baaf57f3d6a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousScanner.java 0ac3df63a260a201544b3623d4901b355d7ea21a
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousUtil.java PRE-CREATION
> src/server/src/main/java/org/apache/accumulo/server/test/continuous/ContinuousWalk.java 7d1e7f95c7e5a7a172990322e2992e536f2725e3
>
> Diff: https://reviews.apache.org/r/18444/diff/
>
>
> Testing
> -------
>
> Unit tests. Started processes on 3 node cluster and batch walkers did not die.
>
>
> Thanks,
>
> Mike Drob
>
>