Accumulo
  1. Accumulo
  2. ACCUMULO-2419

Improve SimpleTimer by replacing java.util.Timer

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7.0
    • Component/s: tserver
    • Labels:

      Description

      The server utility class SimpleTimer uses a java.util.Timer under the hood for scheduling tasks. From Java Concurrency in Practice, p. 123:

      Timer has some drawbacks, and ScheduledThreadPoolExecutor should be thought of as its replacement. ... there is little reason to use Timer in Java 5.0 or later.

      The purpose of SimpleTimer is "to reduce the number of threads dedicated to simple events", but a user cannot opt to let more than one thread handle the events on systems that can take the load. Also, if any task does take a long time for some reason, execution of other tasks is affected.

      The Timer in SimpleTimer should be replaced with ScheduledThreadPoolExecutor, and the class should allow for more than one thread to be used for task execution.

        Issue Links

          Activity

          Bill Havanki created issue -
          Bill Havanki made changes -
          Field Original Value New Value
          Assignee Bill Havanki [ bhavanki ]
          Bill Havanki made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Bill Havanki added a comment -

          Question for the group: I plan to support specifying the thread pool size for SimpleTimer (default 1) with a configuration property, but callers of getInstance(), like TabletServer and Master, will need to pass in a configuration object for that to happen. Is it appropriate to update all of those callers within this ticket - say, as a separate commit - or is that better done under a separate ticket?

          Show
          Bill Havanki added a comment - Question for the group: I plan to support specifying the thread pool size for SimpleTimer (default 1) with a configuration property, but callers of getInstance() , like TabletServer and Master , will need to pass in a configuration object for that to happen. Is it appropriate to update all of those callers within this ticket - say, as a separate commit - or is that better done under a separate ticket?
          Hide
          John Vines added a comment -

          Same commit

          Show
          John Vines added a comment - Same commit
          Bill Havanki made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Bill Havanki made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Bill Havanki made changes -
          Remote Link This issue links to "Review (Web Link)" [ 14526 ]
          Hide
          Bill Havanki added a comment -

          Review up. It looks large-ish, but most of the files changed are replacing SimpleTimer.getInstance() (no-arg) calls with ones that support using multiple threads.

          Show
          Bill Havanki added a comment - Review up. It looks large-ish, but most of the files changed are replacing SimpleTimer.getInstance() (no-arg) calls with ones that support using multiple threads.
          Hide
          ASF subversion and git services added a comment -

          Commit 3bd0caa5877e16fd3ff98863040dc6eb67fdeef2 in accumulo's branch refs/heads/master from Bill Havanki
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=3bd0caa ]

          ACCUMULO-2419 Reimplement SimpleTimer using executors

          The SimpleTimer class is changed to use Java's executor service for thread pools, rather
          than java.util.Timer. This allows Accumulo to run SimpleTimer threads using more than
          one thread, for systems that can handle the load.

          This change also transitions a wide variety of SimpleTimer users to use the new form of the
          SimpleTimer.getInstance() method.

          Show
          ASF subversion and git services added a comment - Commit 3bd0caa5877e16fd3ff98863040dc6eb67fdeef2 in accumulo's branch refs/heads/master from Bill Havanki [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=3bd0caa ] ACCUMULO-2419 Reimplement SimpleTimer using executors The SimpleTimer class is changed to use Java's executor service for thread pools, rather than java.util.Timer. This allows Accumulo to run SimpleTimer threads using more than one thread, for systems that can handle the load. This change also transitions a wide variety of SimpleTimer users to use the new form of the SimpleTimer.getInstance() method.
          Bill Havanki made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          ASF subversion and git services added a comment -

          Commit 2412419afe54d797af59ff9cf1cf182484c14b3c in accumulo's branch refs/heads/master from Eric Newton
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=2412419 ]

          ACCUMULO-2419 remove warnings about accessing a static in a static way

          Show
          ASF subversion and git services added a comment - Commit 2412419afe54d797af59ff9cf1cf182484c14b3c in accumulo's branch refs/heads/master from Eric Newton [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=2412419 ] ACCUMULO-2419 remove warnings about accessing a static in a static way
          John Vines made changes -
          Fix Version/s 1.7.0 [ 12324607 ]
          Mike Drob made changes -
          Link This issue is duplicated by ACCUMULO-2502 [ ACCUMULO-2502 ]

            People

            • Assignee:
              Bill Havanki
              Reporter:
              Bill Havanki
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development