On 10/31/10 12:38 AM, Gary Gregory wrote:
On Oct 31, 2010, at 8:55, "Phil Steitz"<phil.ste...@gmail.com> wrote:
On 10/30/10 10:55 PM, Gary Gregory wrote:
-----Original Message----- From: Phil Steitz
[mailto:phil.ste...@gmail.com] Sent: Sunday, October 31, 2010
06:35 To: Commons Developers List Subject: Re: [pool] Pool
config vs. factory hierarchies.
On 10/30/10 2:30 PM, Simone Tripodi wrote:
Hi Phil, the benefits of eliminating the member variables in
favor of storing pool config reference are IMHO in therms of
code maintainability and keep it as much simple as possible.
Maybe I am being dense here, but I don't quite get that. The
pool has properties such as numActive that it needs to maintain
as well as the config-related properties. Keeping the Config
instance around as a new pool property adds complexity and
doesn't really improve maintainability, as far as I can see;
but it is quite possible I am missing something.
I do not think anyone is being dense here ;) This conversation is
revealing to me that what the pool concepts I have in my head
might be based on incorrect assumptions.
When I initially saw the long list of ivars (13 or so I think)
that GOP and GKOP had in common, that duplication jumped out to
me as duplication. This is the initial issue I was trying to
resolve. It turns out that the 'fix' for that also leaked to the
config classes.
As this thread goes on, it /sounds like/ that the GOP and GKOP
are really completely different beasts, which justify what
/appears/ like duplication. Here, it would help to have ivars and
methods renamed to the best possible names, which would remove
the appearance of duplication.
+1 to look carefully at the full list of member fields and exposed properties
of the GOP and GKOP and decide which ones to rename in GKOP and which ones to
drop or redefine.
Who can do this asap so we can see and understand our pile better ?
Well, all are welcome to dig in and form opinions. It is important
that we get the new API right, so I encourage everyone to do that.
Here is how I see things now.
GOP has the following properties (see javadoc in the 1.x code for
full descriptions)
numActive
numIdle
maxActive
maxIdle
minIdle
maxWait
whenExhaustedAction
minEvictableIdleTimeMillis
softMinEvictableIdleTimeMillis
testOnBorrow
testOnReturn
testWhileIdle
timeBetweenEvictionRunsMillis
numTestsPerEvictionRun
lifo
factory
The first two are initialized at 0 and are read-only. All of the
others are read/write with default values. We have decided to
deprecate the setter for the factory property and we have not
decided yet which others should be non-final. Here is my suggested
list of properties that should be runtime mutable:
maxTotal, maxWait, maxIdle, minIdle, idleTimeout, softIdleTimeout
I propose that we rename maxActive to maxTotal, since the name is
confusing, as it really represents the limit on the total number of
object instances (idle or "active") under management by the pool.
While maxActive does in fact limit numActive, they actually measure
different things, so the names should be different. The others
should be obvious.
GKOP has all of the above properties, but adds
maxTotal
and has parameterized properties
numIdle(key)
numActive(key)
maxActive, maxIdle, minIdle are binding per key.
I propose for GKOP
s/maxActive/maxTotalPerKey
s/maxIdle/maxIdlePerKey
s/minIdle/minIdlePerKey
A radical idea that I have been considering is to propose that we
dispense with keyed pools altogether. The DBCP need can be met
without them (see jdbc-pool) and I often ask why it is worth the
bother maintaining GKOP when users can just manage their own
concurrent maps of GOPs. I guess there is some value to the
maintenance and active/idle accounting across pools; but sometimes I
wonder...
Phil
But I do not buy the completely different beasts argument 100%
because the GKOP pool feels related to the GOP.
They are not completely unrelated, but the differences are in the core methods
both of the pools and their associated object factories.
It would be possible for example, that the GOP subclass GKOP as a
degenerate simple case where the GOP has one pool in a GKOP. That
seems radical, but it would eliminate a lot of apparent code
duplication: public class AltGenericObjectPool<T> extends
GenericKeyedObjectPool<T, T>. That would be weird in the sense
that APIs like borrowObject() and borrowObject(T) would be
available but you get the idea.
I understand the reasoning here, but I don't see it as natural and it would be
hard to implement efficiently (at least I don't see an easy way to do it).
Here again, I could be missing something that would make this easy.
Maybe it's another strategy that could plugged in that would give the pool a
keyed vs not behavior.
Finally, if GOP and GKOP are really separate classes not meant to
share code, then should go back to ivars instead of a config
object in each pool?
I like replacing all of the vars in the ctors with Config instances.
+1. I dislike classes with laundry lists of cries.
What I am not sure adds value is keeping the Config instances as members of the
pool classes
It might be interesting from the pov of being able to answer succinctly: here
is how I was configured. But you could do the same by creating a new config
object and answering it.
I don't like having to maintain similar code in GOP and GKOP and as we think
about how to bring in the jdbc-pool stuff, I would like to see if we can reduce
the amount of similar code in these two classes, or even if there is a way to
somehow combine them efficiently, so I am open to ideas like above.
That is my ideal goal : efficient combo.
GG
Phil
Gary
You can see the difference between the current
Stack(Keyed)ObjectPool(Factory) - which are implemented
according your vision - and Generic(Keyed)ObjectPool(Factory)
implementations - that are still implemented with my first
refactory.
No, these have the same new complexity (vis a vis the original
code) and I am not seeing what the benefit is. Keeping the
pool properties as member fields, making the Config objects
immutable and just having the ctors copy properties from the
immutable Config instances (which are then thrown away) seems
simpler and no harder to maintain to me. Again, I must be
missing something.
I guess at the end of the day, I don't see the "Config"
abstraction as adding anything post-construction - i.e,, this
abstraction is only useful at pool construction time. From the
user perspective, the config properties are no different from
any other pool properties, nor are they different internally.
Why should we need to write Config.maxActive internally, but
just use numActive and other properties directly? This looks
to me like added complexity that does not add any value, at
least for the pools. It makes more sense to me for the
factories to hold a reference to a Config instance.
Phil
On Sat, Oct 30, 2010 at 6:56 PM, Phil
Steitz<phil.ste...@gmail.com> wrote:
On 10/29/10 2:41 PM, James Carman wrote:
On Fri, Oct 29, 2010 at 2:24 PM, sebb<seb...@gmail.com>
wrote:
I had overlooked that aspect ...
If some changes are more expensive to perform, then the
method might want to determine which items have
changed, rather than just reconfiguring everything.
There may be some changes that don't require a pool
update.
Right now, it appears that they just call that "allocate"
method which seems like they kind of "nuke and pave", so
I don't think it's that big of a deal.
Factory reconfig probably just needs to update the
stored config variable, which can be volatile.
I'm not familiar with this factory config stuff. I'll
have to dig in further.
Sorry to be late on this. Here are the requirements:
1. Some subset of the config properties (need to decide
this - should be topic of a different thread) need to be
*individually* mutable at runtime - e.g.,
setMaxActive(newMaxActive) needs to remain. We have agreed
at this point that at least maxActive and maxWait need to
be runtime mutable.
2. Correct functioning of the pool with the current
implementation requires that no thread can change maxActive
while another thread holds the lock on the pool's monitor.
Just making the properties volatile or protecting them with
another lock will cause problems.
I am OK keeping the mutable Config instances around, but I
don't see any real advantage to eliminating the member
variables storing pool config properties - i.e., my
preference would be to make the Config instances immutable
and only used as structs for ctors.
I am +0 on adding a (pool-synchronized) reconfigure(Config)
to enable multiple properties to be changed atomically.
Phil
---------------------------------------------------------------------
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
