Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose AsyncExecutor workload to monitoring via JMX #1542

Merged
merged 1 commit into from
Nov 30, 2016
Merged

Conversation

szeiger
Copy link
Member

@szeiger szeiger commented Jun 30, 2016

  • Parse registerMbeans and poolName options in JdbcBackend and
    document that they apply to all pool implementations, not just
    HikariCP. The AsyncExecutor is now created with the configured
    poolName (instead of the config path) as its name.
  • Register an implementation of the new AsyncExecutorMXBean interface
    as an MBean when creating an AsyncExecutor with registerMbeans=true.
    It is unregistered on close(). Any errors during registration or
    unregistration are logged as errors and ignored.

Fixes #1532. Test in MBeansTest.testMBeans.

- Parse `registerMbeans` and `poolName` options in `JdbcBackend` and
  document that they apply to all pool implementations, not just
  HikariCP. The AsyncExecutor is now created with the configured
  `poolName` (instead of the config path) as its name.

- Register an implementation of the new `AsyncExecutorMXBean` interface
  as an MBean when creating an `AsyncExecutor` with `registerMbeans=true`.
  It is unregistered on `close()`. Any errors during registration or
  unregistration are logged as errors and ignored.

Fixes #1532. Test in MBeansTest.testMBeans.
@szeiger
Copy link
Member Author

szeiger commented Jun 30, 2016

Updated test case to accept queue size 4 as well. After @kwark's comments on #1274 I assumed that a ThreadPoolExecutor would bypass the queue when free threads are available bu after seeing the failures and looking at the implementation, this doesn't seem to be the case. All jobs are enqueued, so we can legitimately see a queue size of 4 instead of 3 under high load.

@szeiger
Copy link
Member Author

szeiger commented Oct 31, 2016

Any takers?

@szeiger
Copy link
Member Author

szeiger commented Nov 15, 2016

@cvogt Can you take a look?

@szeiger
Copy link
Member Author

szeiger commented Nov 30, 2016

Since I can't get anyone to take I look I'll assume there are no objections.

@szeiger szeiger merged commit 074002e into master Nov 30, 2016
@szeiger szeiger deleted the tmp/issue-1532 branch November 30, 2016 12:34
@cvogt
Copy link
Member

cvogt commented Nov 30, 2016

Missed the notification for this. Skimmed the code now. Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose AsyncExecutor stats for monitoring
2 participants