accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Drob" <md...@mdrob.com>
Subject Re: Review Request 18773: ACCUMULO-2429 - Add shell shutdown for thread cleanup
Date Wed, 05 Mar 2014 16:39:34 GMT

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



core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
<https://reviews.apache.org/r/18773/#comment67175>

    Should move the assignment inside of the try-block.



core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java
<https://reviews.apache.org/r/18773/#comment67176>

    Do we need a null check?



test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
<https://reviews.apache.org/r/18773/#comment67174>

    Why did the tracer get removed from here?



test/src/test/java/org/apache/accumulo/test/ShellServerIT.java
<https://reviews.apache.org/r/18773/#comment67177>

    Do we need a null check?


- Mike Drob


On March 5, 2014, 4:29 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18773/
> -----------------------------------------------------------
> 
> (Updated March 5, 2014, 4:29 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2429
>     https://issues.apache.org/jira/browse/ACCUMULO-2429
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> The JLine 2 ConsoleReader used by Shell spawns a thread which should be cleaned up when
done with the Shell. Otherwise, the thread leaks, taking up resources when the shell is used
programmatically. This commit adds a shutdown() method to Shell for cleaning up the thread.
This enables ShellServerIT to pass reliably and not flood the OS with leaked threads.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java 850816c7e1673d89ae8fd29818cf3322c3cd8b3b

>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java df4d817aad50e3c34e6de02f68b4345af00bfac6

>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java 5a6cc8a0052b03cfa510b80b072408139428ccff

>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellTest.java bf203f77d4e6809bacdb82217e3c8d8aaeecbe48

>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java da094e85ed3ebaeef2ac2cac5beb7b52299a49dc

> 
> Diff: https://reviews.apache.org/r/18773/diff/
> 
> 
> Testing
> -------
> 
> Ran shell-related unit tests successfully. Ran ShellServerIT successfully under both
Mac OS X and CentOS 6. Ran shell against live instance to ensure it starts and stops normally.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


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