commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simone Tripodi <simone.trip...@gmail.com>
Subject Re: [pool] Reusing Config
Date Mon, 25 Oct 2010 12:36:24 GMT
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
<GGregory@seagullsoftware.com> wrote:
>> -----Original Message-----
>> From: Steven Siebert [mailto:smsiebe@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
>> <GGregory@seagullsoftware.com>wrote:
>>
>> > > -----Original Message-----
>> > > From: Phil Steitz [mailto:phil.steitz@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.tripodi@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
>> > > > <GGregory@seagullsoftware.com> wrote:
>> > > >>> -----Original Message-----
>> > > >>> From: Simone Tripodi [mailto:simone.tripodi@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
>> > > >>> <GGregory@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: ggregory@seagullsoftware.com
>> > > >>> > Web: seagull.rocketsoftware.com
>> > > >>> >
>> > > >>> >
>> > > >>> >
>> > > >>> >> -----Original Message-----
>> > > >>> >> From: Simone Tripodi [mailto:simone.tripodi@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
>> > > >>> >> <GGregory@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: ggregory@seagullsoftware.com
>> > > >>> >> > Web: seagull.rocketsoftware.com
>> > > >>> >> >
>> > > >>> >> >
>> > > >>> >> >
>> > > >>> >> >> -----Original Message-----
>> > > >>> >> >> From: Simone Tripodi [mailto:simone.tripodi@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
>> > > >>> >> >> <GGregory@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.tripodi@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
>> > > >>> >> >> >> <GGregory@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: ggregory@seagullsoftware.com
>> > > >>> >> >> >>> Web: seagull.rocketsoftware.com
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>> -----Original Message-----
>> > > >>> >> >> >>>> From: Gary Gregory [mailto:GGregory@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:
>> > > >>> >> ggregory@seagullsoftware.com<mailto:ggregory@seagullsoftware.com>
>> > > >>> >> >> >>>> Web:
>> > > >>> >> seagull.rocketsoftware.com<http://www.seagull.rocketsoftware.com/
>> > >
>> > > >>> >> >> >>>>
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>
>> > ----------------------------------------------------------------
>> > > ----
>> > > >>> -
>> > > >>> >> >> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> >> >> >>> For additional commands, e-mail:
>> > dev-help@commons.apache.org
>> > > >>> >> >> >>>
>> > > >>> >> >> >>>
>> > > >>> >> >> >>
>> > > >>> >> >> >>
>> > -----------------------------------------------------------------
>> > > ----
>> > > >>> >> >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> >> >> >> For additional commands, e-mail:
>> > dev-help@commons.apache.org
>> > > >>> >> >> >>
>> > > >>> >> >> >
>> > > >>> >> >> >
>> > ------------------------------------------------------------------
>> > > ---
>> > > >>> >> >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> >> >> > For additional commands, e-mail: dev-help@commons.apache.org
>> > > >>> >> >> >
>> > > >>> >> >> >
>> > > >>> >> >>
>> > > >>> >> >>
>> > --------------------------------------------------------------------
>> > > -
>> > > >>> >> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> >> >> For additional commands, e-mail: dev-help@commons.apache.org
>> > > >>> >> >
>> > > >>> >> >
>> > > >>> >>
>> > > >>> >>
>> > ---------------------------------------------------------------------
>> > > >>> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> >> For additional commands, e-mail: dev-help@commons.apache.org
>> > > >>> >
>> > > >>> >
>> > > >>>
>> > > >>> ---------------------------------------------------------------------
>> > > >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > >>> For additional commands, e-mail: dev-help@commons.apache.org
>> > > >>
>> > > >>
>> > > >
>> > > > ---------------------------------------------------------------------
>> > > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > > For additional commands, e-mail: dev-help@commons.apache.org
>> > > >
>> > > >
>> > >
>> > > ---------------------------------------------------------------------
>> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> > For additional commands, e-mail: dev-help@commons.apache.org
>> >
>> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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


Mime
View raw message