add metrics support (Codahale)

[vladmihalcea]

add PoolingDataSource connection wait time histogram

add PoolingDataSource in-use connections histrogram

[brettwooldridge]

Vlad, so you consider this pull request ready? If so, I will start reviewing it in preparation for merging.

[vladmihalcea]

Hi,

I think it is quite ready. I started a new "enhancement" in an attempt to find the right pool size.

This is the project: https://github.com/vladmihalcea/flexy-pool

I think it's also worth to have dedicated exceptions on BTM too.

So instead of BitronixRuntimeException("XA pool of resource ... still empty after ... wait time") we could have a ConnectionAcquireTimeoutException

What do you think?

Vlad

On Friday, March 21, 2014 10:52 AM, Brett Wooldridge [email protected] wrote:

Vlad, so you consider this pull request ready? If so, I will start reviewing it in preparation for merging.
—
Reply to this email directly or view it on GitHub.

[lorban]

Vlad,

Throwing more specific exceptions could be done, obviously as long as that
doesn't break anything. I'd recommend you to open a feature request and
start working on it in a branch if you don't mind.

BTW, you should probably add support for Brett's HikariCP connection pool
to your flexy-pool tool. :-)

Ludovic

On Fri, Mar 21, 2014 at 11:05 AM, Vlad Mihalcea [email protected]:

Hi,

I think it is quite ready. I started a new "enhancement" in an attempt to
find the right pool size.

This is the project: https://github.com/vladmihalcea/flexy-pool

I think it's also worth to have dedicated exceptions on BTM too.

So instead of BitronixRuntimeException("XA pool of resource ... still
empty after ... wait time") we could have a
ConnectionAcquireTimeoutException

What do you think?

Vlad

On Friday, March 21, 2014 10:52 AM, Brett Wooldridge <
[email protected]> wrote:

Vlad, so you consider this pull request ready? If so, I will start

reviewing it in preparation for merging.

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com//pull/12#issuecomment-38262406
.

[lorban]

Why extending Serializable here?

[vladmihalcea]

It should be removed.

[rankinc]

Just to mention: we also use Codahale metrics in our BTM application, except we package them inside the WAR file. So abstracting the metrics from BTM strikes me as a Good Thing, because otherwise we would need to change how we deploy everything :-).

[vladmihalcea]

Things to be done:

1. TransactionManagerServices should expose the MetricsFactory is similar fashion with other Bitronix services
2. Metrics are domain bound, so we only need one Metrics instance per current DataSource. The TransactionManagerServices should register all Metrics and close them during shutdown.
3. No inner-class instantiator which is not thread-safe.
4. Remove the serializable marker from Metrics.
5. Rework unit tests to accommodate the new design.

I tried a first attempt to change those and tests were failing. I will try to find a way to fix those in the following weeks.