Added metrics support (using Codahale metrics).

[vladmihalcea]

Added CodaHale Metrics support
Added PoolingDataSource connection wait time metric
Added PoolingDataSource in-use-connections histogram metric

Refactored the in-use-connections logic since previous code review, please check its logic out.
Basically it should tell how many connections are in use at any given time, for building an accurate histogram.
My current solution is based on subtracting: totalPoolSize - availablePoolSize.

[vladmihalcea]

I had to delete my previous repository, because due to reverts, the rebase was not working. So I recreated my master, and pushed this single commit.

Even if I commit temporary changes and squash them prior to sending the pull-request, I still need to push, as GitHub requires the changes in the remote repository.

But if the code review requires new commits, I could then simply commit in my local branch and re-squash the already squashed commit (the one that was reviewed) with the latest changes (addressing the code review).

Can I create feature branches with GitHub fork repositories too, so I can take advantage of the simplified pull-request UI?

[brettwooldridge]

Yes, once you fork, you can create any branches you like in your own fork. See here about how to push a local feature branch to your git repository (and track it).

[vladmihalcea]

Hi,

That's how I intend to work on external repositories from now on.

I found this free online book(http://git-scm.com/book) about Git, which details all workflows.
And this guide is very well written: http://blog.anvard.org/conversational-git/.

There is a lot of documentation for Git and GitHub, but every project has to define its own workflow.
Usually, when you start using GitHub, you choose the master branch workflow, which makes sense if you have
a small project with a limited number of contributors.

For larger projects like Bitronix, the feature branch approach makes more sense, but it's not that easy, unless you have
some prior experience with Git.

That's why I thought of writing down a step-by-step visual guide.

I'll send it to you when it's done and if you like it, you can add it to some developer guideline document.

Vlad

On Tuesday, February 11, 2014 12:38 PM, Brett Wooldridge notifications@github.com wrote:

Yes, once you fork, you can create any branches you like in your own fork. See here about how to push a local feature branch to your git repository (and track it).
—
Reply to this email directly or view it on GitHub.

[brettwooldridge]

I also tend to like the feature-branch model, where reviewed contributions
are merged into master, rather than pull requests coming from a forked
master. I've worked on several other projects that use it (pgjdbc-ng comes
to mind).

Brett

On Tue, Feb 11, 2014 at 7:51 PM, Vlad Mihalcea notifications@github.comwrote:

Hi,

That's how I intend to work on external repositories from now on.

I found this free online book(http://git-scm.com/book) about Git, which
details all workflows.
And this guide is very well written:
http://blog.anvard.org/conversational-git/.

There is a lot of documentation for Git and GitHub, but every project has
to define its own workflow.
Usually, when you start using GitHub, you choose the master branch
workflow, which makes sense if you have
a small project with a limited number of contributors.

For larger projects like Bitronix, the feature branch approach makes more
sense, but it's not that easy, unless you have
some prior experience with Git.

That's why I thought of writing down a step-by-step visual guide.

I'll send it to you when it's done and if you like it, you can add it to
some developer guideline document.

Vlad

On Tuesday, February 11, 2014 12:38 PM, Brett Wooldridge <
notifications@github.com> wrote:

Yes, once you fork, you can create any branches you like in your own fork.
See here about how to push a local feature branch to your git repository

(and track it).

Reply to this email directly or view it on GitHub.

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

[brettwooldridge]

... The only tricky part is, if the btm/master "moves" while you are
working on your feature branch, the feature-branch should be rebased
against the master before the pull request is submitted, otherwise it is
messy.

On Tue, Feb 11, 2014 at 7:53 PM, Brett Wooldridge <
brett.wooldridge@gmail.com> wrote:

I also tend to like the feature-branch model, where reviewed contributions
are merged into master, rather than pull requests coming from a forked
master. I've worked on several other projects that use it (pgjdbc-ng comes
to mind).

Brett

On Tue, Feb 11, 2014 at 7:51 PM, Vlad Mihalcea notifications@github.comwrote:

Hi,

That's how I intend to work on external repositories from now on.

I found this free online book(http://git-scm.com/book) about Git, which
details all workflows.
And this guide is very well written:
http://blog.anvard.org/conversational-git/.

There is a lot of documentation for Git and GitHub, but every project has
to define its own workflow.
Usually, when you start using GitHub, you choose the master branch
workflow, which makes sense if you have
a small project with a limited number of contributors.

For larger projects like Bitronix, the feature branch approach makes more
sense, but it's not that easy, unless you have
some prior experience with Git.

That's why I thought of writing down a step-by-step visual guide.

I'll send it to you when it's done and if you like it, you can add it to
some developer guideline document.

Vlad

On Tuesday, February 11, 2014 12:38 PM, Brett Wooldridge <
notifications@github.com> wrote:

Yes, once you fork, you can create any branches you like in your own
fork. See here about how to push a local feature branch to your git

repository (and track it).

Reply to this email directly or view it on GitHub.

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

[vladmihalcea]

Hi,

But you can use the pull-request just for code-reviewing only and merge it with Git only when you want to add it to a specific release.

This way you(the product owner) don't have to use the original Pull-request, you can just merge the contributor's feature branch into your release branch (the product owner's master)
whenever you need it. You can move your master anyway you like, since it's the product owner that decides where to take the changes from and to what branch to add them.

When I started using GitHub, I relied only on the UI for all my work. With feature branches, you can forget about it and use only the command line. The web UI is useful for code reviewing.

Vlad

On Tuesday, February 11, 2014 12:55 PM, Brett Wooldridge notifications@github.com wrote:

... The only tricky part is, if the btm/master "moves" while you are
working on your feature branch, the feature-branch should be rebased
against the master before the pull request is submitted, otherwise it is
messy.

On Tue, Feb 11, 2014 at 7:53 PM, Brett Wooldridge <
brett.wooldridge@gmail.com> wrote:

I also tend to like the feature-branch model, where reviewed contributions
are merged into master, rather than pull requests coming from a forked
master. I've worked on several other projects that use it (pgjdbc-ng comes
to mind).

Brett

On Tue, Feb 11, 2014 at 7:51 PM, Vlad Mihalcea notifications@github.comwrote:

Hi,

That's how I intend to work on external repositories from now on.

I found this free online book(http://git-scm.com/book) about Git, which
details all workflows.
And this guide is very well written:
http://blog.anvard.org/conversational-git/.

There is a lot of documentation for Git and GitHub, but every project has
to define its own workflow.
Usually, when you start using GitHub, you choose the master branch
workflow, which makes sense if you have
a small project with a limited number of contributors.

For larger projects like Bitronix, the feature branch approach makes more
sense, but it's not that easy, unless you have
some prior experience with Git.

That's why I thought of writing down a step-by-step visual guide.

I'll send it to you when it's done and if you like it, you can add it to
some developer guideline document.

Vlad

On Tuesday, February 11, 2014 12:38 PM, Brett Wooldridge <
notifications@github.com> wrote:

Yes, once you fork, you can create any branches you like in your own
fork. See here about how to push a local feature branch to your git

repository (and track it).

Reply to this email directly or view it on GitHub.

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

—
Reply to this email directly or view it on GitHub.

[lorban]

I've just reviewed your Metrics changes. Except for the initialization code that has to be rewritten to be in TransactionManagerServices and to be thread-safe, I quite like it!

If you can do those modifications, I'll merge your branch in the official source tree. If you need help or something is unclear, just let me know.
Thanks!

[vladmihalcea]

Thanks for appreciating it. I'll have to find some time to fix those findings and commit them in my branch, then squash and force them. I'll let you know when I'm done.

On Wednesday, April 2, 2014 11:11 AM, lorban notifications@github.com wrote:

I've just reviewed your Metrics changes. Except for the initialization code that has to be rewritten to be in TransactionManagerServices and to be thread-safe, I quite like it!
If you can do those modifications, I'll merge your branch in the official source tree. If you need help or something is unclear, just let me know.
Thanks!
—
Reply to this email directly or view it on GitHub.

[vladmihalcea]

Your review made me realized the metrics design doesn't fit into the current Bitronix architecture. I am on a tight schedule so I am not sure when I'll be able to rewrite the metrics implementation.

I'll summarize it to know what's to do:

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 accomodate the new design.

Vlad

On Wednesday, April 2, 2014 2:14 PM, Mihalcea Vlad mih_vlad@yahoo.com wrote:

Thanks for appreciating it. I'll have to find some time to fix those findings and commit them in my branch, then squash and force them. I'll let you know when I'm done.

On Wednesday, April 2, 2014 11:11 AM, lorban notifications@github.com wrote:

I've just reviewed your Metrics changes. Except for the initialization code that has to be rewritten to be in TransactionManagerServices and to be thread-safe, I quite like it!
If you can do those modifications, I'll merge your branch in the official source tree. If you need help or something is unclear, just let me know.
Thanks!
—
Reply to this email directly or view it on GitHub.