You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Bill Havanki <bh...@clouderagovt.com> on 2014/03/07 17:52:19 UTC

Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

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

Review request for accumulo.


Bugs: ACCUMULO-2419
    https://issues.apache.org/jira/browse/ACCUMULO-2419


Repository: accumulo


Description
-------

A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.

Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.

Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.


Diffs
-----

  core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
  minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
  server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
  server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
  server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
  server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
  server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
  server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
  server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
  server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
  server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
  server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
  server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
  server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
  test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 

Diff: https://reviews.apache.org/r/18909/diff/


Testing
-------

SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.


Thanks,

Bill Havanki

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/#review36533
-----------------------------------------------------------



server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java
<https://reviews.apache.org/r/18909/#comment67532>

    Need to specify that behavior for null config is to use default of one thread.


- Bill Havanki


On March 7, 2014, 11:52 a.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 11:52 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/
-----------------------------------------------------------

(Updated March 14, 2014, 2:27 p.m.)


Review request for accumulo.


Changes
-------

Refactoring based on Keith's feedback.

The changes reported in diff 3 for TabletServerResourceManager are spurious, from a separate issue committed since March 7 and not part of the code under review.


Bugs: ACCUMULO-2419
    https://issues.apache.org/jira/browse/ACCUMULO-2419


Repository: accumulo


Description
-------

A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.

Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.

Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.


Diffs (updated)
-----

  core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
  minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
  server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
  server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
  server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
  server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
  server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
  server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
  server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
  server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
  server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
  server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
  server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
  server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java 4eae109adf7506f0f24418350d30cf2ae010f3ce 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
  test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 

Diff: https://reviews.apache.org/r/18909/diff/


Testing
-------

SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.


Thanks,

Bill Havanki

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.

> On March 13, 2014, 6:31 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java, line 107
> > <https://reviews.apache.org/r/18909/diff/2/?file=513827#file513827line107>
> >
> >     why not call genInstance(int)?

Yup. I didn't notice the refactor opportunity during the work. New diff coming...


- Bill


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


On March 7, 2014, 2:46 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 2:46 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/#review37120
-----------------------------------------------------------



server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java
<https://reviews.apache.org/r/18909/#comment68462>

    why not call genInstance(int)?


- kturner


On March 7, 2014, 7:46 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 7:46 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/#review37094
-----------------------------------------------------------

Ship it!


Ship It!

- Mike Drob


On March 7, 2014, 7:46 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 7:46 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/
-----------------------------------------------------------

(Updated March 7, 2014, 2:46 p.m.)


Review request for accumulo.


Changes
-------

Included madrob's suggestions.
- better log message when task throws exception
- Javadoc about passing null config
- Javadoc and log warning about using a different thread pool count after the singleton is made
- removed SimpleTimer.shutdown()
- more unit tests


Bugs: ACCUMULO-2419
    https://issues.apache.org/jira/browse/ACCUMULO-2419


Repository: accumulo


Description
-------

A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.

Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.

Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.


Diffs (updated)
-----

  core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
  minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
  server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
  server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
  server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
  server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
  server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
  server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
  server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
  server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
  server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
  server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
  server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
  server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
  test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 

Diff: https://reviews.apache.org/r/18909/diff/


Testing
-------

SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.


Thanks,

Bill Havanki

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.

> On March 7, 2014, 1:04 p.m., Mike Drob wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java, line 38
> > <https://reviews.apache.org/r/18909/diff/1/?file=513673#file513673line38>
> >
> >     Logging both the message and the exception is generally duplicative, as the framework should print the message before the stack trace anyway.

Good point, will fix.


> On March 7, 2014, 1:04 p.m., Mike Drob wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java, line 74
> > <https://reviews.apache.org/r/18909/diff/1/?file=513673#file513673line74>
> >
> >     If I call getInstance(1) and later getInstance(2) then that behaviour is not clear from an API perspective. Properties beginning with general.* usually can't be changed at runtime though, so we might be ok.

True. The behavior is that the later call will return the singleton instance with only one thread. I can add a log message warning about this case? I should also note it in the Javadoc.


> On March 7, 2014, 1:04 p.m., Mike Drob wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java, line 132
> > <https://reviews.apache.org/r/18909/diff/1/?file=513673#file513673line132>
> >
> >     I don't see this getting called anywhere. Do we need to retrofit existing classes to clean up after themselves?

I added this anticipating a desire for it, but you are correct, it's not used. Actually, I added it before I changed the threads to be daemons, so I can remove it.


> On March 7, 2014, 1:04 p.m., Mike Drob wrote:
> > server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java, line 24
> > <https://reviews.apache.org/r/18909/diff/1/?file=513675#file513675line24>
> >
> >     Add a test to check exceptions?

Sure, I'll see what I can add.


- Bill


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


On March 7, 2014, 11:52 a.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 11:52 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Bill Havanki <bh...@clouderagovt.com>.

> On March 7, 2014, 1:04 p.m., Mike Drob wrote:
> > server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java, line 24
> > <https://reviews.apache.org/r/18909/diff/1/?file=513675#file513675line24>
> >
> >     Add a test to check exceptions?
> 
> Bill Havanki wrote:
>     Sure, I'll see what I can add.

I added a couple of tests, one of which ensures that if the task throws an exception things are still OK. I didn't see any other spots where an exception test would be super useful, but if you see one, point it out and I'll code one up.


- Bill


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


On March 7, 2014, 2:46 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 2:46 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Re: Review Request 18909: ACCUMULO-2419 - Change SimpleTimer to use ScheduledThreadPoolExecutor

Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18909/#review36541
-----------------------------------------------------------



server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java
<https://reviews.apache.org/r/18909/#comment67543>

    Logging both the message and the exception is generally duplicative, as the framework should print the message before the stack trace anyway.



server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java
<https://reviews.apache.org/r/18909/#comment67545>

    If I call getInstance(1) and later getInstance(2) then that behaviour is not clear from an API perspective. Properties beginning with general.* usually can't be changed at runtime though, so we might be ok.



server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java
<https://reviews.apache.org/r/18909/#comment67546>

    I don't see this getting called anywhere. Do we need to retrofit existing classes to clean up after themselves?



server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java
<https://reviews.apache.org/r/18909/#comment67547>

    Add a test to check exceptions?


- Mike Drob


On March 7, 2014, 4:52 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18909/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 4:52 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2419
>     https://issues.apache.org/jira/browse/ACCUMULO-2419
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> A conversion from SimpleTimer using java.util.Timer to java.util.concurrent.ScheduledThreadPoolExecutor.
> 
> Most of the key changes are in SimpleTimer.java. It now can support more than one thread; the number of threads is specified either directly or through the new general.server.simpletimer.threadpool.size configuration property. The default is one thread, as with Timer. Submitted tasks are no longer decorated with LoggingTimerTask, but run as-is, with a thread exception handler taking over logging of errors. A unit test written before the conversion ensures that SimpleTimer still works.
> 
> Other changes are there to move away from calling the no-arg SimpleTimer.getInstance(), so that the prevailing configuration is consulted to figure out the SimpleTimer thread count.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java efa7eb58f9d38ba6d68b70b502353ad5a8899ba2 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 35a7b9d2889d34ef0d323713b004e9d1d3b25989 
>   server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa905113caa583241cf88afc6c9d39a467bfedc 
>   server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java 63bd894e1e3122b11074074455be7fbfe5043736 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 6d9e4c7735a4fe72312b2e5640dc8a59916cade1 
>   server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java 499b0de4f505d62ae48272c93a9901a0e6642746 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/DistributedWorkQueue.java c5a952827e0944f62b227b10b0504af7ce9496be 
>   server/base/src/test/java/org/apache/accumulo/server/util/time/SimpleTimerTest.java PRE-CREATION 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 89925b46a77a978982208f8117f6ec38281450e3 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 8e98c0421d219b34c0306465ae5f65fab7b94a00 
>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java 76d352059fa3bce0fff1370dc982fa56da50c450 
>   server/master/src/main/java/org/apache/accumulo/master/tableOps/BulkImport.java bdc89dd1140ac1cba822f96c613112b1b5c9fa75 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/BasicServlet.java bf65dae1fe9dd0fab68be8b67965460bb4990809 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java 30f1ae768a9c61464eda991eed94de8e74f39dca 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionWatcher.java e6ca38feff9920d10136540c7163cda2d5d3b1f8 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java bb95532589b6a5476459fadf54131b968882915d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java db046e9da55ff66a223d34bb832c495157a25ee6 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java 8f783c330acfcc7599468bf269fb70c8e2a828b7 
>   test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java f26c8d7fcaf9695f04a64b2668ab2d37afded0df 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 0591b19b83a9a3a9b1cf62a80f967fa633a2520e 
> 
> Diff: https://reviews.apache.org/r/18909/diff/
> 
> 
> Testing
> -------
> 
> SimpleTimer unit test succeeds. Ran functional tests, most succeeded (the few failures are unrelated to SimpleTimer). Ran test cluster with changes, did basic table operations.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>