You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mark Thomas <ma...@apache.org> on 2007/12/02 19:09:09 UTC
[POOL] Offer of help for a 1.4 release
Hi,
Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we are
considering reverting to pool-1.2 but would obviously prefer to move to
pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
A quick scan of the archives suggests that an offer of an extra pair of
hands might help speed up a pool-1.4 release in the near term. So, here I am.
I am happy to help out in what ever way I can with a pool-1.4 release.
Anything from providing doc patches to actually rolling the release. I
accept that you probably don't want to trust a newcomer with rolling a
release - I just wanted you to know the level of commitment I am prepared
to make (and will to continue to make after the 1.4 release) to get this fixed.
Let me know what I can do to help. The only string attached to this offer
is the fact I want to see a pool-1.4 release asap.
Cheers,
Mark
[1] http://issues.apache.org/bugzilla/show_bug.cgi?id=43552
[2] https://issues.apache.org/jira/browse/POOL-97
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Christoph Kutzinski <ku...@gmx.de>.
Phil Steitz wrote:
>>>> If you are really worried about the cost of object creation then you can
>>>> configure the pool to create all the objects at start-up and block until a
>>>> free object is available.
>>
>> That is unfortunately not possible under our current configuration as we
>> have set up our application servers to use all connections our database
>> server can handle when their pools have reached their maximum size.
>> For example: we have 40 application servers with a pool max-size of 40.
>> Our database server can just handle (because of its memory
>> configuration) 1600 connections.
>> If we would configure the pools to fetch all connections at startup, we
>> would lose the ability to do updates to our application-software (we
>> have a 2-stage approach to doing updates: we startup the 2nd stage with
>> the new software then configure the load balancer to use the 2nd stage
>> and only afterwards stop the 1st stage) without major hassle.
>>
> So I guess you use the idle object evictor to trim back the
> connections in use by the pools and do the maintenance under low load
> conditions where you can count on only about 50% max connection
> utilization? Am I following this correctly? I am not sure I follow
> what you are doing here.
Yes, we use minIdle and softMinEvictableIdleTimeMillis (which should be
the default IMHO, since I don't see any sensible use for
minEvictableIdleTimeMillis. But that's a different story) to trim down
the pool to 10 connections, which is enough under normal load
conditions. So we have now problems to do updates at that moment.
When we are getting into high load conditions the pool size is ramping
up over several hours until it reaches 40.
>> I still think that serial connection creation is a good thing as it will
>> help to keep unnecessary load from the database server:
>> As connections borrowed from the pool are held only for a comparably
>> short time (at least in our case), the probability that a connection was
>> returned to pool by a different thread in the near future is quite high.
>>
>> So, by serializing connection creation and rechecking, if a connection
>> is available, before starting to create a new one, you won't burden the
>> db-server with unnecessary load.
>>
>
> The 1.2 / 1.4-RC1 code does "recheck" before initiating additional
> makes - i.e., it will not initiate a makeObject if an idle object has
> been returned to the pool or if maxActive has been reached. I think I
> understand your point though, but again it doesn't seem natural to use
> client thread synchronization in the connection pool as a load
> dampening mechanism for the database.
Where is this recheck? I can't see it - all I see is an initial check,
but no re-check. I doubt that there can be one in 1.4 RC1 as (as far as
I see), makeObjects are fully parallel.
I see your point however that I'm trying to use the conn-pool as a load
throttling mechanism for the db, which I cannot deny. I agree that it is
questionable if this is a job, which a connection pool should do.
>
>> Another thing to consider: If the db-server is under high load, creating
>> connections in parallel probably won't give you any time benefits. While
>> in idle mode it may be true that:
>> When I get 1 connection in 100 ms, I also get n (say 4) connections in
>> ~100ms
>> under high load situations it is much different as all processors on the
>> db-server are busy with other jobs. So it will probably look much more
>> like this:
>> 1 connection in 2 seconds
>> 2 connections in 4 seconds
>> ...
>
> I would be interested to see real data on this and also impacts of
> connection request load spikes on various engines (i.e., how well they
> handle bursts of "simultaneous" connection requests).
I'll see if I can generate a test scenario with our JMeter load tests to
get at least some data for our environment.
> The engines are
> going to end up queueing them anyway and it may be better to leave
> that concern to them.
That's a very good point (though I think that's just a speculation that
the engines are queueing them anyway). However I would be still
concerned in that case that connections, which are not needed anymore,
are created. Consider this:
Threads A and B are requesting new connection creations. The requests
are queued up (by the db engine). When the connection for Thread A is
created, Thread C has already returned his connection to the pool, so B
could use that one. But since the request for a new connection was
already issued to the db engine, it will be created now anyway.
Another thing we should consider, is the question what you are trying to
achieve by using a pool. I can see 2 main points:
I) providing 'objects' fast when they are requested
II) avoiding (unnecessary) load on the engine providing the objects
(e.g. the DB server)
To some degree both points are important, but different use cases might
have different priorities:
a) if load on the db server is not the main problem (it has enough power
to handle even the highest peak with ease), providing connections as
fast as possible might be the priority. So the pool should create as
many connections in parallel as are requested.
b) if load on the db matters much and the speed by which connections are
returned from the pool are not the main problem, you probably want to
serialize connection creation
In our use case, we are focussed on II and b, since we are trying to
keep our db server responsive for as long as possible under peak loads.
Different applications may be more focussed on I and a, so my proposed
changes might be contraproductive for them.
Christoph
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
On Jan 5, 2008 7:49 AM, <ku...@gmx.de> wrote:
> Hi Phil,
>
> thanks for taking my issue serious.
>
> Phil Steitz wrote:
>
> > Thanks again for the feedback. Any other opinions / suggestions on
> > this are appreciated. I suggest the following compromise, which would
> > also fix the maxActive exceeded by one issue I discovered with
> > 1.2/1.4-RC1 yesterday:
> >
> > (*) Move makeObject back inside synchronized scope in borrowObject.
> > (patch below)
> >
> > This addresses Christoph's production use case (assuming I have
> > understood it correctly) and also closes the maxActive exceeded by one
> > problem that my testing uncovered yesterday. It does not have a huge
> > performance impact in the tests that I have run and still leaves
> > activation and validation (the per-request operations) outside of
> > synchronized scope. More elegant solutions to both problems are
> > certainly possible and we can consider them in subsequent releases -
> > including configurable serialization of just the makes.
>
> This is not what I was trying to achieve :-(
> While this fixes my issue with the parallel creation of objects, it
> reopens another issue which I have with the implementation in pool 1.3:
>
> The pool blocks completely while an object creation is in progress.
> Just consider the following use case:
>
> - Thread A tries to borrow an object. The pool contains no idle objects,
> hence a new one is created
> - While object is still in creation, Thread B tries to return his
> borrowed object to the pool. Since returning the object depends on the
> same synchronization lock as makeObject, B will block as long as the new
> object isn't created (which can maybe take several seconds, as seen in
> my use case). I think this is unacceptable.
Agreed. This is why the synchronization was removed. Bad idea to
stay with the 1.3 setup.
> - Still while the creation is in progress, Thread C wants to borrow an
> object. It cannot continue, either, though an idle object would be
> available, if B could have returned its object
>
> I propose, as in my previous post, a distinct lock for object creation.
> I have attached a patch which should fix this behavior, but probably
> will not fix the "maxActive exceeded by 1". Do you already have a
> testcase for this one?
I am working on this now. I don't have a unit test yet. I discovered
the problem using load tests generated using the performance module in
commons-sandbox. The dummy factory that the pool tests use keeps
track of (makes - destroys) and throws IllegalStateException if this
quantity exceeds maxActive for the pool. I was able to get this to
happen using the config I posted on the other thread.
>The existing unit tests run without errors when
> my patch is applied.
>
Thanks! Testing now.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by ku...@gmx.de.
Hi Phil,
thanks for taking my issue serious.
Phil Steitz wrote:
> Thanks again for the feedback. Any other opinions / suggestions on
> this are appreciated. I suggest the following compromise, which would
> also fix the maxActive exceeded by one issue I discovered with
> 1.2/1.4-RC1 yesterday:
>
> (*) Move makeObject back inside synchronized scope in borrowObject.
> (patch below)
>
> This addresses Christoph's production use case (assuming I have
> understood it correctly) and also closes the maxActive exceeded by one
> problem that my testing uncovered yesterday. It does not have a huge
> performance impact in the tests that I have run and still leaves
> activation and validation (the per-request operations) outside of
> synchronized scope. More elegant solutions to both problems are
> certainly possible and we can consider them in subsequent releases -
> including configurable serialization of just the makes.
This is not what I was trying to achieve :-(
While this fixes my issue with the parallel creation of objects, it
reopens another issue which I have with the implementation in pool 1.3:
The pool blocks completely while an object creation is in progress.
Just consider the following use case:
- Thread A tries to borrow an object. The pool contains no idle objects,
hence a new one is created
- While object is still in creation, Thread B tries to return his
borrowed object to the pool. Since returning the object depends on the
same synchronization lock as makeObject, B will block as long as the new
object isn't created (which can maybe take several seconds, as seen in
my use case). I think this is unacceptable.
- Still while the creation is in progress, Thread C wants to borrow an
object. It cannot continue, either, though an idle object would be
available, if B could have returned its object
I propose, as in my previous post, a distinct lock for object creation.
I have attached a patch which should fix this behavior, but probably
will not fix the "maxActive exceeded by 1". Do you already have a
testcase for this one? The existing unit tests run without errors when
my patch is applied.
Greetings
Christoph
Index: src/java/org/apache/commons/pool/impl/GenericObjectPool.java
===================================================================
--- src/java/org/apache/commons/pool/impl/GenericObjectPool.java
(revision 609143)
+++ src/java/org/apache/commons/pool/impl/GenericObjectPool.java
(working copy)
@@ -322,6 +322,8 @@
*/
public static final long
DEFAULT_SOFT_MIN_EVICTABLE_IDLE_TIME_MILLIS = -1;
+ private final Object makeObjectLock = new Object();
+
//--- constructors -----------------------------------------------
/**
@@ -920,7 +922,7 @@
} catch(NoSuchElementException e) {
; /* ignored */
}
-
+
// otherwise
if(null == pair) {
// check if we can create one
@@ -969,19 +971,21 @@
// create new object when needed
boolean newlyCreated = false;
if(null == pair) {
- try {
- Object obj = _factory.makeObject();
- pair = new ObjectTimestampPair(obj);
- newlyCreated = true;
- } finally {
- if (!newlyCreated) {
- // object cannot be created
- synchronized (this) {
- _numActive--;
- notifyAll();
- }
- }
- }
+ synchronized( makeObjectLock ) {
+ try {
+ Object obj = _factory.makeObject();
+ pair = new ObjectTimestampPair(obj);
+ newlyCreated = true;
+ } finally {
+ if (!newlyCreated) {
+ // object cannot be created
+ synchronized (this) {
+ _numActive--;
+ notifyAll();
+ }
+ }
+ }
+ }
}
// activate & validate the object
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
On Jan 4, 2008 4:09 AM, <ku...@gmx.de> wrote:
> Mark, Thomas, thanks for your replies,
>
> Phil Steitz wrote:
> > On Jan 3, 2008 12:40 PM, Mark Thomas <ma...@apache.org> wrote:
> >> Christoph Kutzinski wrote:
> >>> - creating a new object means the pool is exhausted which in turn usually means that we have a high-load situation.
> >>> - creation of new objects is expensive (probably even more in high-load situations). This is why we originally used the pool
> >>> - so in conclusion it is probably a bad idea to create multiple object in parallel
> >> I don't see how serializing object creation can help performance. If you
> >> have a test case and some numbers that show otherwise, I would be very
> >> interested in taking a look.
>
> I have no test case for this, I just have my reasoning and my
> observation on our live system that connection creation (lets call the
> 'objects' database connections as that is probably 95% of the use cases
> of commons-pool :-) ) takes much longer under high load situations:
> While connection creation in 'idle' mode takes something between 100 and
> 200 ms, it takes several seconds (the longest I've seen was 27 seconds!)
> under peak loads.
>
>
> >>
> >> If you are really worried about the cost of object creation then you can
> >> configure the pool to create all the objects at start-up and block until a
> >> free object is available.
>
>
> That is unfortunately not possible under our current configuration as we
> have set up our application servers to use all connections our database
> server can handle when their pools have reached their maximum size.
> For example: we have 40 application servers with a pool max-size of 40.
> Our database server can just handle (because of its memory
> configuration) 1600 connections.
> If we would configure the pools to fetch all connections at startup, we
> would lose the ability to do updates to our application-software (we
> have a 2-stage approach to doing updates: we startup the 2nd stage with
> the new software then configure the load balancer to use the 2nd stage
> and only afterwards stop the 1st stage) without major hassle.
>
So I guess you use the idle object evictor to trim back the
connections in use by the pools and do the maintenance under low load
conditions where you can count on only about 50% max connection
utilization? Am I following this correctly? I am not sure I follow
what you are doing here.
>
>
>
> > Thanks for the feedback, Christoph; but I agree with Mark. I suspect
> > most pool users keep the default whenExhaustedAction, which is to
> > block. That means that objects get created a) on demand, when there
> > are no idle instances, but maxActive has not been reached b) when
> > addObject is invoked to prefill or augment the idle object pool
> > explicitly or c) when minIdle is set and the idle object evictor runs.
> > Even when a) happens during a load spike, it is better to do the
> > makes in parallel, especially if there is some latency involved and
> > there are resources available to process the makes in parallel (which
> > will be the case in, e.g. a database connection pool).
>
> Phil, I cannot follow your reasoning here. What makes you think that
> there are "resources available to process the makes in parallel"? What
> resources do you think of anyway? I'm thinking about resources as
> processor-cycles on the database server and these are usually not
> available during peak load times.
>
I understand now what you meant. What I meant was really that the
operation could be done in parallel.
> I still think that serial connection creation is a good thing as it will
> help to keep unnecessary load from the database server:
> As connections borrowed from the pool are held only for a comparably
> short time (at least in our case), the probability that a connection was
> returned to pool by a different thread in the near future is quite high.
>
> So, by serializing connection creation and rechecking, if a connection
> is available, before starting to create a new one, you won't burden the
> db-server with unnecessary load.
>
The 1.2 / 1.4-RC1 code does "recheck" before initiating additional
makes - i.e., it will not initiate a makeObject if an idle object has
been returned to the pool or if maxActive has been reached. I think I
understand your point though, but again it doesn't seem natural to use
client thread synchronization in the connection pool as a load
dampening mechanism for the database.
> Another thing to consider: If the db-server is under high load, creating
> connections in parallel probably won't give you any time benefits. While
> in idle mode it may be true that:
> When I get 1 connection in 100 ms, I also get n (say 4) connections in
> ~100ms
> under high load situations it is much different as all processors on the
> db-server are busy with other jobs. So it will probably look much more
> like this:
> 1 connection in 2 seconds
> 2 connections in 4 seconds
> ...
I would be interested to see real data on this and also impacts of
connection request load spikes on various engines (i.e., how well they
handle bursts of "simultaneous" connection requests). The engines are
going to end up queueing them anyway and it may be better to leave
that concern to them.
>
> So in this case serial creation might be even better from the
> application side of view, as you already have 1 connection after 2
> seconds (the 2nd after 4, and so on), while using parallel creation you
> would have to wait 4 seconds to get a connection.
>
>
> After all these are all considerations by me without any hard evidence
> supporting it, besides some observations I made in our live system
> during peak loads. So you can reject them, if you don't think that they
> would hold for the majority for the pool clients out there. But I still
> think that for our use case this would be the best way to proceed.
>
Thanks again for the feedback. Any other opinions / suggestions on
this are appreciated. I suggest the following compromise, which would
also fix the maxActive exceeded by one issue I discovered with
1.2/1.4-RC1 yesterday:
(*) Move makeObject back inside synchronized scope in borrowObject.
(patch below)
This addresses Christoph's production use case (assuming I have
understood it correctly) and also closes the maxActive exceeded by one
problem that my testing uncovered yesterday. It does not have a huge
performance impact in the tests that I have run and still leaves
activation and validation (the per-request operations) outside of
synchronized scope. More elegant solutions to both problems are
certainly possible and we can consider them in subsequent releases -
including configurable serialization of just the makes.
If there are no objections and my full suite of soak tests succeed and
do not show bad performance, I will proceed with this change and roll
RC2 this weekend.
Phil
-----------------------------------------------
Index: src/java/org/apache/commons/pool/impl/GenericObjectPool.java
===================================================================
--- src/java/org/apache/commons/pool/impl/GenericObjectPool.java (revision
608225)
+++ src/java/org/apache/commons/pool/impl/GenericObjectPool.java (working copy)
@@ -911,7 +911,7 @@
long starttime = System.currentTimeMillis();
for(;;) {
ObjectTimestampPair pair = null;
-
+ boolean newlyCreated = false;
synchronized (this) {
assertOpen();
// if there are any sleeping, just grab one of those
@@ -964,19 +964,16 @@
}
}
_numActive++;
- }
- // create new object when needed
- boolean newlyCreated = false;
- if(null == pair) {
- try {
- Object obj = _factory.makeObject();
- pair = new ObjectTimestampPair(obj);
- newlyCreated = true;
- } finally {
- if (!newlyCreated) {
- // object cannot be created
- synchronized (this) {
+ // create new object when needed
+ if(null == pair) {
+ try {
+ Object obj = _factory.makeObject();
+ pair = new ObjectTimestampPair(obj);
+ newlyCreated = true;
+ } finally {
+ if (!newlyCreated) {
+ // object cannot be created
_numActive--;
notifyAll();
}
Phil.
>
> Christoph
>
>
> ---------------------------------------------------------------------
> 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
Re: [POOL] Offer of help for a 1.4 release
Posted by ku...@gmx.de.
Mark, Thomas, thanks for your replies,
Phil Steitz wrote:
> On Jan 3, 2008 12:40 PM, Mark Thomas <ma...@apache.org> wrote:
>> Christoph Kutzinski wrote:
>>> - creating a new object means the pool is exhausted which in turn usually means that we have a high-load situation.
>>> - creation of new objects is expensive (probably even more in high-load situations). This is why we originally used the pool
>>> - so in conclusion it is probably a bad idea to create multiple object in parallel
>> I don't see how serializing object creation can help performance. If you
>> have a test case and some numbers that show otherwise, I would be very
>> interested in taking a look.
I have no test case for this, I just have my reasoning and my
observation on our live system that connection creation (lets call the
'objects' database connections as that is probably 95% of the use cases
of commons-pool :-) ) takes much longer under high load situations:
While connection creation in 'idle' mode takes something between 100 and
200 ms, it takes several seconds (the longest I've seen was 27 seconds!)
under peak loads.
>>
>> If you are really worried about the cost of object creation then you can
>> configure the pool to create all the objects at start-up and block until a
>> free object is available.
That is unfortunately not possible under our current configuration as we
have set up our application servers to use all connections our database
server can handle when their pools have reached their maximum size.
For example: we have 40 application servers with a pool max-size of 40.
Our database server can just handle (because of its memory
configuration) 1600 connections.
If we would configure the pools to fetch all connections at startup, we
would lose the ability to do updates to our application-software (we
have a 2-stage approach to doing updates: we startup the 2nd stage with
the new software then configure the load balancer to use the 2nd stage
and only afterwards stop the 1st stage) without major hassle.
> Thanks for the feedback, Christoph; but I agree with Mark. I suspect
> most pool users keep the default whenExhaustedAction, which is to
> block. That means that objects get created a) on demand, when there
> are no idle instances, but maxActive has not been reached b) when
> addObject is invoked to prefill or augment the idle object pool
> explicitly or c) when minIdle is set and the idle object evictor runs.
> Even when a) happens during a load spike, it is better to do the
> makes in parallel, especially if there is some latency involved and
> there are resources available to process the makes in parallel (which
> will be the case in, e.g. a database connection pool).
Phil, I cannot follow your reasoning here. What makes you think that
there are "resources available to process the makes in parallel"? What
resources do you think of anyway? I'm thinking about resources as
processor-cycles on the database server and these are usually not
available during peak load times.
I still think that serial connection creation is a good thing as it will
help to keep unnecessary load from the database server:
As connections borrowed from the pool are held only for a comparably
short time (at least in our case), the probability that a connection was
returned to pool by a different thread in the near future is quite high.
So, by serializing connection creation and rechecking, if a connection
is available, before starting to create a new one, you won't burden the
db-server with unnecessary load.
Another thing to consider: If the db-server is under high load, creating
connections in parallel probably won't give you any time benefits. While
in idle mode it may be true that:
When I get 1 connection in 100 ms, I also get n (say 4) connections in
~100ms
under high load situations it is much different as all processors on the
db-server are busy with other jobs. So it will probably look much more
like this:
1 connection in 2 seconds
2 connections in 4 seconds
...
So in this case serial creation might be even better from the
application side of view, as you already have 1 connection after 2
seconds (the 2nd after 4, and so on), while using parallel creation you
would have to wait 4 seconds to get a connection.
After all these are all considerations by me without any hard evidence
supporting it, besides some observations I made in our live system
during peak loads. So you can reject them, if you don't think that they
would hold for the majority for the pool clients out there. But I still
think that for our use case this would be the best way to proceed.
Christoph
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
On Jan 3, 2008 12:40 PM, Mark Thomas <ma...@apache.org> wrote:
> Christoph Kutzinski wrote:
> > - creating a new object means the pool is exhausted which in turn usually means that we have a high-load situation.
> > - creation of new objects is expensive (probably even more in high-load situations). This is why we originally used the pool
> > - so in conclusion it is probably a bad idea to create multiple object in parallel
>
> I don't see how serializing object creation can help performance. If you
> have a test case and some numbers that show otherwise, I would be very
> interested in taking a look.
>
> If you are really worried about the cost of object creation then you can
> configure the pool to create all the objects at start-up and block until a
> free object is available.
>
Thanks for the feedback, Christoph; but I agree with Mark. I suspect
most pool users keep the default whenExhaustedAction, which is to
block. That means that objects get created a) on demand, when there
are no idle instances, but maxActive has not been reached b) when
addObject is invoked to prefill or augment the idle object pool
explicitly or c) when minIdle is set and the idle object evictor runs.
Even when a) happens during a load spike, it is better to do the
makes in parallel, especially if there is some latency involved and
there are resources available to process the makes in parallel (which
will be the case in, e.g. a database connection pool). This is all
assuming that there is good synchronized control over the number of
makes. (See post to follow.). As Mark said, if makes are *very*
expensive, you can prefill and then configure the pool to block.
Phil
> ---------------------------------------------------------------------
> 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
Re: [POOL] Offer of help for a 1.4 release
Posted by Mark Thomas <ma...@apache.org>.
Christoph Kutzinski wrote:
> - creating a new object means the pool is exhausted which in turn usually means that we have a high-load situation.
> - creation of new objects is expensive (probably even more in high-load situations). This is why we originally used the pool
> - so in conclusion it is probably a bad idea to create multiple object in parallel
I don't see how serializing object creation can help performance. If you
have a test case and some numbers that show otherwise, I would be very
interested in taking a look.
If you are really worried about the cost of object creation then you can
configure the pool to create all the objects at start-up and block until a
free object is available.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Christoph Kutzinski <ku...@gmx.de>.
Hi,
sorry for the late reply, but I've been very busy in the last weeks.
I still wanted to make some comments on the changes to the synchronization in GenericObjectPool (and that like) - not sure which JIRA number refers to that.
I think one of the few good things about the strong synchronization in pool's 1.3 implementation was the fact that the creation of new objects was serialised. This is gone with the patch, i.e. multiple objects may be created in parallel. Let me explain why I think this is bad:
- creating a new object means the pool is exhausted which in turn usually means that we have a high-load situation.
- creation of new objects is expensive (probably even more in high-load situations). This is why we originally used the pool
- so in conclusion it is probably a bad idea to create multiple object in parallel
My suggestion is to protect the creation of new objects with a different lock to serialise it. E.g. (sketch):
// create new object when needed
if(null == pair) {
synchronized(makeObjectLock) {
Object obj = _factory.makeObject();
...
}
}
Usage of a different lock ensures that other operations on the pool don't block because of object creation in progress.
Additionally you probably want to recheck if an object was returned to the pool while you were waiting on the lock:
if(null == pair) {
synchronized(makeObjectLock) {
if( objectIsStillNotAvailable() ) {
Object obj = _factory.makeObject();
...
} else {
// return obj from pool
}
}
}
If you want to make the serialisation on the object creation configurable you can even add a method to provide the lock:
synchronized( getMakeObjectLock() ) {
...
}
private Object getMakeObjectLock() {
if( this.serialObjectCreation ) {
return this.makeObjectLock;
} else {
return new Object();
}
}
greetings
Christoph
-------- Original-Nachricht --------
> Datum: Sun, 9 Dec 2007 20:35:28 -0700
> Von: "Phil Steitz" <ph...@gmail.com>
> An: "Jakarta Commons Developers List" <de...@commons.apache.org>
> Betreff: Re: [POOL] Offer of help for a 1.4 release
> On Dec 9, 2007 4:46 PM, Mark Thomas <ma...@apache.org> wrote:
> > Mark Thomas wrote:
> > > Phil Steitz wrote:
> > >> On Dec 2, 2007 11:09 AM, Mark Thomas <ma...@apache.org> wrote:
> > >>> Hi,
> > >>>
> > >>> Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we
> are
> > >>> considering reverting to pool-1.2 but would obviously prefer to move
> to
> > >>> pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
> > >>>
> > >>> A quick scan of the archives suggests that an offer of an extra pair
> of
> > >>> hands might help speed up a pool-1.4 release in the near term. So,
> here I am.
> > >> Thanks! This is exactly what is needed.
> > >
> > > OK. I have trunk checked out and building. I'll starting looking at
> > > POOL-86, POOL-97, POOL-93 & POOL-108 in terms of reviewing what is
> there
> > > and developing patches where there are gaps. I'll need to get my head
> > > around the pool code so I might be quiet for a couple of days.
> > >
> > > I'll add JIRA comments when I have something useful to say.
> >
> > I have taken a look at each of the issues above. Comments added to JIRA.
> In
> > summary:
>
> Thanks!
>
> > POOL-86 - The patch in JIRA looks good to me
> To be clear, are you referring to the patch that was applied in
> rr582538. - i.e., the code in trunk? (thanks, btw, for completing
> some cleanup missed there :)
>
> > POOL-93 - Proposed improved patch based on patches in JIRA
> Committed.
>
> > POOL-97 - Proposed alternative patch that fixes 97 without a regression
>
> Committed.
>
> > POOL-108 - Proposed improved patch (see POOL-93) based on patch in JIRA
> >
> > I'm happy to re-work any/all of the patches if required.
>
> Only rework required will be if we decide to roll back or modify the
> POOL-86 change.
>
> Thanks again for your help on this.
>
> Phil
>
> ---------------------------------------------------------------------
> 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
Re: [POOL] Offer of help for a 1.4 release
Posted by Mark Thomas <ma...@apache.org>.
Phil Steitz wrote:
> On Dec 9, 2007 4:46 PM, Mark Thomas <ma...@apache.org> wrote:
>> I have taken a look at each of the issues above. Comments added to JIRA. In
>> summary:
>
> Thanks!
>
>> POOL-86 - The patch in JIRA looks good to me
> To be clear, are you referring to the patch that was applied in
> rr582538. - i.e., the code in trunk? (thanks, btw, for completing
> some cleanup missed there :)
Sorry for not being clear. Yes, it was r582538 I reviewed.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
On Dec 9, 2007 4:46 PM, Mark Thomas <ma...@apache.org> wrote:
> Mark Thomas wrote:
> > Phil Steitz wrote:
> >> On Dec 2, 2007 11:09 AM, Mark Thomas <ma...@apache.org> wrote:
> >>> Hi,
> >>>
> >>> Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we are
> >>> considering reverting to pool-1.2 but would obviously prefer to move to
> >>> pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
> >>>
> >>> A quick scan of the archives suggests that an offer of an extra pair of
> >>> hands might help speed up a pool-1.4 release in the near term. So, here I am.
> >> Thanks! This is exactly what is needed.
> >
> > OK. I have trunk checked out and building. I'll starting looking at
> > POOL-86, POOL-97, POOL-93 & POOL-108 in terms of reviewing what is there
> > and developing patches where there are gaps. I'll need to get my head
> > around the pool code so I might be quiet for a couple of days.
> >
> > I'll add JIRA comments when I have something useful to say.
>
> I have taken a look at each of the issues above. Comments added to JIRA. In
> summary:
Thanks!
> POOL-86 - The patch in JIRA looks good to me
To be clear, are you referring to the patch that was applied in
rr582538. - i.e., the code in trunk? (thanks, btw, for completing
some cleanup missed there :)
> POOL-93 - Proposed improved patch based on patches in JIRA
Committed.
> POOL-97 - Proposed alternative patch that fixes 97 without a regression
Committed.
> POOL-108 - Proposed improved patch (see POOL-93) based on patch in JIRA
>
> I'm happy to re-work any/all of the patches if required.
Only rework required will be if we decide to roll back or modify the
POOL-86 change.
Thanks again for your help on this.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Mark Thomas <ma...@apache.org>.
Mark Thomas wrote:
> Phil Steitz wrote:
>> On Dec 2, 2007 11:09 AM, Mark Thomas <ma...@apache.org> wrote:
>>> Hi,
>>>
>>> Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we are
>>> considering reverting to pool-1.2 but would obviously prefer to move to
>>> pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
>>>
>>> A quick scan of the archives suggests that an offer of an extra pair of
>>> hands might help speed up a pool-1.4 release in the near term. So, here I am.
>> Thanks! This is exactly what is needed.
>
> OK. I have trunk checked out and building. I'll starting looking at
> POOL-86, POOL-97, POOL-93 & POOL-108 in terms of reviewing what is there
> and developing patches where there are gaps. I'll need to get my head
> around the pool code so I might be quiet for a couple of days.
>
> I'll add JIRA comments when I have something useful to say.
I have taken a look at each of the issues above. Comments added to JIRA. In
summary:
POOL-86 - The patch in JIRA looks good to me
POOL-93 - Proposed improved patch based on patches in JIRA
POOL-97 - Proposed alternative patch that fixes 97 without a regression
POOL-108 - Proposed improved patch (see POOL-93) based on patch in JIRA
I'm happy to re-work any/all of the patches if required.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Mark Thomas <ma...@apache.org>.
Phil Steitz wrote:
> On Dec 2, 2007 11:09 AM, Mark Thomas <ma...@apache.org> wrote:
>> Hi,
>>
>> Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we are
>> considering reverting to pool-1.2 but would obviously prefer to move to
>> pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
>>
>> A quick scan of the archives suggests that an offer of an extra pair of
>> hands might help speed up a pool-1.4 release in the near term. So, here I am.
>
> Thanks! This is exactly what is needed.
OK. I have trunk checked out and building. I'll starting looking at
POOL-86, POOL-97, POOL-93 & POOL-108 in terms of reviewing what is there
and developing patches where there are gaps. I'll need to get my head
around the pool code so I might be quiet for a couple of days.
I'll add JIRA comments when I have something useful to say.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
> I think it would
> really be best to resolve the over-synch issues as well if we can do
> it quickly. I will post a separate note on what I have in mind there.
See patch and comment on POOL-108.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [POOL] Offer of help for a 1.4 release
Posted by Phil Steitz <ph...@gmail.com>.
On Dec 2, 2007 11:09 AM, Mark Thomas <ma...@apache.org> wrote:
> Hi,
>
> Tomcat has been bitten [1] by a bug [2] in pool-1.3. Currently we are
> considering reverting to pool-1.2 but would obviously prefer to move to
> pool-1.4 (that included a fix for [2]) due to the many fixes in 1.3.
>
> A quick scan of the archives suggests that an offer of an extra pair of
> hands might help speed up a pool-1.4 release in the near term. So, here I am.
Thanks! This is exactly what is needed. Some issues have been
resolved since 1.3, but more review is needed to validate the changes
(e.g. fixes for POOL-86). Other than this, the key issues that we
need to address are POOL-97 (the one that you refer to below) and the
over-synchronization of borrowObject (POOL-108, POOL-93), which was
introduced in 1.3. I have been contemplating reverting to the 1.2
synchronization setup. This is an area where more eyeballs / ideas
would be very helpful. The LIFO/FIFO issue mentioned below (POOL-86)
has been addressed in trunk. I will do the RM work if we can get at
least POOL-97 closed (any ideas you have on this would be greatly
appreciated) and the fixes for POOL-86 reviewed. I think it would
really be best to resolve the over-synch issues as well if we can do
it quickly. I will post a separate note on what I have in mind there.
Thanks!
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org