On 10/25/10 10:51 AM, Steven Siebert wrote:
Simo,

You make a good point, especially with the groupId/package change.

POOL-169 did implement some deprecations for changes from 1.5 to 2.0, so
this isn't without precedent and consistency (even for the sake of
consistency in commons-* subprojects, IMO) is important.  Honestly, I could
go either way...just wanted to make sure it was considered =)


I agree with both of you. Probably best to give the signal, consistently with what we have been doing up to now. More information to help people plan for the upgrade is better.

Phil
S

On Mon, Oct 25, 2010 at 10:02 AM, Simone Tripodi
<simone.trip...@gmail.com>wrote:

Hi Steven :)
thanks for the explanation, as far as I understand there are a lot of
things I can learn from you about JMX so I started feeling impatient
to see your commits :P

I don't think we should mark with @Deprecate removed ctors, classes
are now living in a fresh new package so, as user, I wouldn't expect
retro-compatibility at all. When upgrading a major revision, I expect
a completely (or partially, in that case) new version of the
library... maybe I'm wrong but my instinct suggests me a 2.0 version
could be even a completely rewrite of the 1.5.
BTW that's just my opinion :P

Have a nice day,
Simo

http://people.apache.org/~simonetripodi/
http://www.99soft.org/



On Mon, Oct 25, 2010 at 3:51 PM, Steven Siebert<smsi...@gmail.com>  wrote:
Simone,

I'm sorry I'm confusing, so many thoughts going though my mind =)

yes, I think the best approach is to provide a separate class for the
JMX...but I'm thinking that doing a private inner class (non-static) in
each
pool would be the cleanest way to do so.  This way, the MBean (instance)
would have direct access to both the config values (read/write) as well
as
the ability to invoke pool methods (such as getNumwaiters(), POOL-159).
This would significantly cut down on the bootstrapping of having to
register
the config and pool with the MBean.

Once I get home, I'll attach a JMX patch to POOL-172 using your latest
commit.

I noticed your two JIRA tickets for the concurrency and builder
pattern....should one be submitted to add the @deprecated tag to the 1.5
API
for the various classes, methods, and constructors we're blowing away?

On Mon, Oct 25, 2010 at 9:30 AM, Simone Tripodi<
simone.trip...@gmail.com>wrote:

Hi Steven,
very sorry to have missed the Jira notifications, just checked now
after read your email. Sorry! :(

I thought the idea was adding JMX support directly on factory/pool and
not on Configuration, btw not being JMX expert this will be a good
chance to learn... I'll fill a new Jira issue for adding thread safety
support on configuration classes, and start to study how to do it in
the best way.

I like the Builder idea, my +1 for that, take it in consideration as
already done :P

Have a nice day and thanks for the feedbacks!
Simo

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Mon, Oct 25, 2010 at 3:06 PM, Steven Siebert<smsi...@gmail.com>
wrote:
Hi Simone,

You have two +1's waiting for you in the JIRA comments =)

My comments from tracker:

"I took a look at this last night but didn't get a chance to comment
=)

I like the patch, I believe this does indeed satisfy the issue.

One question I have, since we're eliminating the primitive
configuration
properties within the pool/factory classes, we're making the Config
objects
publicly accessible, and possibly accessing through JMX is the idea of
making the Config objects thread-safe. This would certainly reduce the
need
to have to externally synchronize (and possibly introduce bugs) every
time.

Another issue we probably need to open another ticket for is to
deprecate
the constructors we've eliminated in 1.5.

Last suggestion/question is about making inner (public static final)
Builder
pattern classes within the concrete Config classes (and possibly
defining
an
abstract<T extends Abstract*Config>  create() method in the Abstract
class).
This would further simplify the programmatic creation of the Config
classes.

Thoughts?

+1 on Simones patch...we can add any of the above after it has been
committed."


Respectfully,


Steve



On Mon, Oct 25, 2010 at 8:36 AM, Simone Tripodi<
simone.trip...@gmail.com>wrote:

Hi all mates,
I updated the jira issue uploading my patch; it contains the
configuration extraction and some code modification.
IMHO we shouldn't replicate the same data in both configuration AND
factory/pool, when creating the factory/pool it is enough storing the
configuration reference, just use it.
I intentionally missed the interfaces layer, since they can be added
directly in the JMX support in the required form.
Please take a look at the patch and provide feedbacks, if you agree I
could start committing the modifications and proceed on JMX support.
Have a nice day,
Simo

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Fri, Oct 22, 2010 at 5:23 AM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
-----Original Message-----
From: Steven Siebert [mailto:smsi...@gmail.com]
Sent: Thursday, October 21, 2010 18:08
To: Commons Developers List
Subject: Re: [pool] Reusing Config

Gary,

Great work so far.  I'm checking out the diffs now, I'm gonna hack
out
some
simple UML "diffs", if only to wrap my head around it all. I'll
upload
the
file to the issue once complete.

BTW, I hope I didn't offend with the 'academic' comment, I
most certainly did not intend to infer that there weren't
functional
importances to this issue.  I was mostly trying to delineate the
two
issues
in my mind, and putting it to "paper" was a good way to do that =)

Cheers,

S

Hi Steven,

No offense even considered from this end :)

I'm glad we are going through this exercise. This will improve the
software I am sure.

Gary



On Thu, Oct 21, 2010 at 3:35 PM, Gary Gregory
<ggreg...@seagullsoftware.com>wrote:

-----Original Message-----
From: Phil Steitz [mailto:phil.ste...@gmail.com]
Sent: Thursday, October 21, 2010 06:29
To: Commons Developers List
Subject: Re: [pool] Reusing Config

On 10/21/10, Simone Tripodi<simone.trip...@gmail.com>  wrote:
it seems you've been doing a very good work, the only thing
I
*suggest*
is

* simplifying the mutable/immutable interfaces, one
interface
for
already known common (im)mutable fields should be enough;
* adding/renaming the interfaces with the<PoolName>`MBean`
postfix to
be ready for JMX support;

btw it seems you're now much more deep inside the topic than
me
;)

WDYT?
Simo


Sorry I have been a little slow on this.  I will have a
careful
look
this eve.  Based on a very quick review, I am +1 on the idea
and
approach to separate mutable / immutable.  Also +1 for JMX
support.
Two quick things to keep top of mind:

1.  Please make sure not to lose documentation.  Whatever is
documented today in protected field / internal getters /
setters
docs
needs to be carried forward.

Check. I did not check as I refactored that Javadocs were in the
right
places. That would be a requirement for a real patch. I only
meant
this as
an experiment that went a lot further than I thought.


2. Somewhat related - I am fine just plowing ahead for now
using
existing API concepts, but some of those concepts are
anachronistic
or
broken, IMO, so we may later decide to revamp much of the
"accounting"
aspects of the  API.  That we should and will discuss on other
threads.  One thing that might be good to think about at this
point,
however, is getting rid of primitive properties (we started
that
with
whenExhaustedAction).  I think there is a DBCP issue on this
raised
by
Dain a couple of years ago.

It would be nice to track this someplace, I am not sure if
Javadoc
is
the
right place.

Gary


Thanks all for moving this along!

Phil
http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Thu, Oct 21, 2010 at 8:23 AM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
-----Original Message-----
From: Simone Tripodi [mailto:simone.trip...@gmail.com]
Sent: Wednesday, October 20, 2010 22:41
To: Commons Developers List
Subject: Re: [pool] Reusing Config

Hi Gary!
unfortunately the link replied with 404 code, can you give
me
please
the issue ID?

It's https://issues.apache.org/jira/browse/POOL-173

I've updated the diff file a couple of times since my
initial
msg.

Gary

Many thanks in advance, have a nice day!!!
Simo

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Thu, Oct 21, 2010 at 12:16 AM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
Hi Simone,

Please see my experiment in progress here




https://issues.apache.org/jira/secure/attachment/12457710/pool2config.diff

Gary Gregory
Senior Software Engineer
Rocket Software
3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 * USA
Tel: +1.404.760.1560
Email: ggreg...@seagullsoftware.com
Web: seagull.rocketsoftware.com



-----Original Message-----
From: Simone Tripodi [mailto:simone.trip...@gmail.com]
Sent: Wednesday, October 20, 2010 14:53
To: Commons Developers List
Subject: Re: [pool] Reusing Config

Hi,
sorry for not having been clear, but in my previous
email
my
intent
was saying that depending on how we manage the Config
class,
it
could
influence de JMX support design, nothing more, and
since
I'm
not
expert on JMX I was waiting for feedbacks from who
knows
more
than
me

About Gary's question, I had the following thought

AbstractGenericObjectPoolConfig
- int maxIdle
- int minIdle
- int maxActive
- long maxWait
- WhenExhaustedAction whenExhaustedAction
- boolean testOnBorrow
- boolean testOnReturn
- boolean testWhileIdle
- long timeBetweenEvictionRunsMillis
- int numTestsPerEvictionRun
- long minEvictableIdleTimeMillis
- boolean lifo

GenericObjectPoolConfig extends
AbstractGenericObjectPoolConfig
- long softMinEvictableIdleTimeMillis

GenericKeyedObjectPoolConfig extends
GenericObjectPoolConfig
- int maxTotal

About the pools:

class GenericObjectPool {
   + GenericObjectPool(GenericObjectPoolFactory factory)
{
       this(factory, new GenericObjectPoolConfig());
   }

   + GenericObjectPool(GenericObjectPoolFactory factory,
GenericObjectPoolConfig config) {...}

   + GenericObjectPoolConfig getConfig() {...}
}

same thing for the Keyed version.

Too simple and stupid? Maybe. But reduces the
redundancies
to
0.
Moreover I'm not sure it is just an academical way to
approach the
issue, I'm sure it is more than pragmatic, simplifying
the
maintainability and makes easier keep in synch the Pool
and
related
Factory configuration.
Just my 2 cents, now off to bed due my local timezone
:P
Simo

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Wed, Oct 20, 2010 at 10:40 PM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
So I am doing an experimental refactoring to see what
the
code
would
look
like with a Config class extracted and I ran into the
following.

The class GenericObjectPool has an
_softMinEvictableIdleTimeMillis
ivar
but
the equivalent GenericKeyedObjectPool does not.

Is that a little hole in implementation that could
have
been
avoided
with
a
common classes used for config? Even if
GenericKeyedObjectPool
would
throw
a
"not implemented" exception.

Thoughts?

Gary Gregory
Senior Software Engineer
Rocket Software
3340 Peachtree Road, Suite 820 * Atlanta, GA 30326 *
USA
Tel: +1.404.760.1560
Email: ggreg...@seagullsoftware.com
Web: seagull.rocketsoftware.com



-----Original Message-----
From: Simone Tripodi [mailto:
simone.trip...@gmail.com]
Sent: Wednesday, October 20, 2010 12:22
To: Commons Developers List
Subject: Re: [pool] Reusing Config

sure, I always wait for feedbacks before coding :P
Cool
expression
"Rambo through the code", that was the first time I
read
it and
made
me laugh :D
All the best,
Simo

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Wed, Oct 20, 2010 at 9:17 PM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
It seems to me there is a reason the code is the
way
it
is so
I'd
really
like to hear thoughts from some of the original
authors
before
we
go and
Rambo
through the code ;)

Gary

On Oct 20, 2010, at 12:13, "Simone Tripodi"
<simone.trip...@gmail.com>
wrote:

Hi Gary,
yes that's me that raised the question[1] and
discussed
a
little
with
Seb. What blocked me was the JMX support proposal
since
I'm
not
familiar with that technology, so I was
consulting
documentation
to
study.

My very big +1 for that, with the wish of work
directly
on
that
stuff.
Anyone else has a different thought, before
proceeding?
Thanks in advance,
Simo

[1] http://markmail.org/message/q4y7ghux57s7hk6v

http://people.apache.org/~simonetripodi/<
http://people.apache.org/%7Esimonetripodi/>
<http://people.apache.org/%7Esimonetripodi/>
http://www.99soft.org/



On Wed, Oct 20, 2010 at 7:43 PM, Gary Gregory
<ggreg...@seagullsoftware.com>  wrote:
In the same department, I see the following
ivars:

lifo : boolean
maxActive : int
maxIdle : int
maxTotal : int
maxWait : long
minEvictableIdleTimeMillis : long
minIdle : int
numTestsPerEvictionRun : int
testOnBorrow : boolean
testOnReturn : boolean
testWhileIdle : boolean
timeBetweenEvictionRunsMillis : long
whenExhaustedAction : WhenExhaustedAction

defined in four classes:

GenericKeyedObjectPool
GenericKeyedObjectPoolFactory
GenericObjectPool
GenericObjectPoolFactory

Which feels to me like a missed opportunity to
avoid
duplication.

Is making one ivar private or final or volatile
be
applied
to
all
four
classes?

We could:

Use a config object instead of the 13 ivars.
Or a common superclass then we can consider if
it
should
hold
the
ivar
list or a Config object.

Would it be too weird to have a common super
class
for
BaseObjectPool
and
BasePoolableObjectFactory for example?

Gary Gregory
Senior Software Engineer
Rocket Software
3340 Peachtree Road, Suite 820 . Atlanta, GA
30326
.
USA
Tel: +1.404.760.1560
Email: ggreg...@seagullsoftware.com
Web: seagull.rocketsoftware.com



-----Original Message-----
From: Gary Gregory [mailto:
ggreg...@seagullsoftware.com]
Sent: Wednesday, October 20, 2010 10:29
To: Commons Developers List
Subject: [pool] Reusing Config

Hi All:

I think this came up recently. Any thoughts or
plans
on
extracting
the
Config
class out of GenericKeyedObjectPool and
GenericObjectPool
so
it can
be
reused.
The constants for default values could then
also
be
moved
to
Config.
Gary Gregory
Senior Software Engineer
Rocket Software
3340 Peachtree Road, Suite 820 * Atlanta, GA
30326
*
USA
Tel: +1.404.760.1560
Email:
ggreg...@seagullsoftware.com<mailto:
ggreg...@seagullsoftware.com>
Web:
seagull.rocketsoftware.com<
http://www.seagull.rocketsoftware.com/





----------------------------------------------------------------
----
-
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org





-----------------------------------------------------------------
----
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org




------------------------------------------------------------------
---
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org





--------------------------------------------------------------------
-
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org





---------------------------------------------------------------------
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail:
dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail:
dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to