On 10/25/10 11:26 AM, Gary Gregory wrote:
Thank you for working through this Simone.
I would like to discuss something I took for granted in my experimental patch
for [POOL-173]. I can see that you took and a more conservative (and safer ;)
approach in your version. I am glad to see this because we can now more easily
discuss it because it is obvious in the code now, where we have the following
config hierarchy:
AbstractGenericObjectPoolConfig
GenericKeyedObjectPoolConfig
GenericObjectPoolConfig
IMO, there should be one Config class (call it GenericObjectPoolConfig for
example.) This hierarchy reflects that the actual two generic pool classes are
out of sync.
For example, why should softMinEvictableIdleTimeMillis exist in
GenericObjectPool but not in GenericKeyedObjectPoolConfig?
When I think about a keyed pool vs. not, I think about a map of pools vs. a
single pool. It does not make sense that they the have the different
configurations as reflected by the current hierarchy.
Thoughts?
Good point, Gary. The softMin property could and probably should be
extended to GKOP. The latter does have one property though -
maxTotal - that only makes sense for a keyed pool.
Unfortunately, another problem that we really have here is that
several of the config parameters mean different things for keyed vs
non-keyed pools. For example, maxActive in a keyed pool really
means max active *per key*. This is why maxTotal has to exist.
Same holds for maxIdle (but there is no "maxTotalIdle"). I have
thought about suggesting that we rename these parameters to
maxActivePerKey and maxIdlePerKey, resp. Then we could dispense
with maxTotal (and keep maxIdle meaning for the union). We might
want to think about doing that (though enforcing maxIdle so defined
will add a little overhead). The nice thing about having the
hierarchy is that that is an easy change at this point (while we are
breaking stuff ;).
Phil
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: Monday, October 25, 2010 05:36
To: Commons Developers List
Subject: Re: [pool] Reusing Config
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://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://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://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://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://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://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
