You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Filip Hanik - Dev Lists <de...@hanik.com> on 2007/04/03 19:26:38 UTC

[DBCP] Periodic clean up of abandoned connections

Would you guys be open to a patch to implement a periodic cleanup of 
abandoned connections
the following logic can cause abandoned connections to sit around 
forever, just because you are not hitting the max in the pool.
Using a DB like postgres that creates a process for each connection, 
this is a waste of resources, and the only way to trigger the abandoned 
cleanup is to try to get to max number of connections.

        if (config != null
                && config.getRemoveAbandoned()
                && (getNumIdle() < 2)
                && (getNumActive() > getMaxActive() - 3) ) {
            removeAbandoned();
        }

Let me know, and if yes, I will provide a patch

Filip

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


Re: [DBCP] Periodic clean up of abandoned connections

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Filip Hanik - Dev Lists wrote:
> Phil Steitz wrote:
>> On 4/3/07, Filip Hanik - Dev Lists <de...@hanik.com> wrote:
>>> Would you guys be open to a patch to implement a periodic cleanup of
>>> abandoned connections
>>> the following logic can cause abandoned connections to sit around
>>> forever, just because you are not hitting the max in the pool.
>>> Using a DB like postgres that creates a process for each connection,
>>> this is a waste of resources, and the only way to trigger the abandoned
>>> cleanup is to try to get to max number of connections.
>>>
>>>         if (config != null
>>>                 && config.getRemoveAbandoned()
>>>                 && (getNumIdle() < 2)
>>>                 && (getNumActive() > getMaxActive() - 3) ) {
>>>             removeAbandoned();
>>>         }
>>>
>>> Let me know, and if yes, I will provide a patch
>>>
>>
>> I agree that the above setup is not optimal.  Now is a good time to
>> talk about what we want to do in this area.  The abandoned connection
>> cleanup problem has sort of a rich history in [dbcp] and is a complex
>> problem.  See for example:
>>
>> http://marc.info/?t=105633413400001&r=1&w=4
>> (Start with David Graham's first post on 24-Jun-2003)
> I actually wrote my own connection pool in 1999 that has abandoned 
> support and this has never been a problem.
> I started reading the thread, and forgive my ignorance, the thread is 
> way too long for me to churn through.
> The solution to a problem like this is quite simple
> 1. When connections are marked abandoned, the wrapper connection is 
> simply marked closed, and the underlying JDBC connection is closed or 
> returned to the pool with a new wrapper.
> 2. When applications are accessing their "java.sql.Connection" method, 
> it simply throws SQLException("Connection closed-abandoned");
> There is 0 concurrency issues.
> I will look into the DBCP code to see if there are any logistics issue 
> specifically to DBCP, but in my implementation it was a piece of cake.
> <5 seconds later>
> Ok, so it looks like the DBCP is not using any "wrapper" connection, 
> hence everyone worries about the "connection" being references all 
> over the placed.
> Well, piece of cake, add a wrapper using a reflection.proxy, and the 
> mystery is solved.
oh, it is using a wrapper, the wrapper could be better though, instead 
of implementing every single java.sql.Connection method, use a proxy, 
and that way support all versions of JDK, as methods do get added 
between JDK versions

Filip
>
> Furthermore, in order to save threads, the generic pool can have a 
> public void tasks(){} method, that the a thread invokes.
> In the GenericPool the tasks() would look like this
> public void tasks() {
> if (_evictor!=null) evictor.run();//of course evictor-runnable is 
> modified to not sleep or delay
> }
>
> The abandoned pool would then override it like this
> public void tasks() {
>  super.tasks();
>  if ( getActiveCount()>0) removeAbandoned();
> }
>
> Is there something apparent I am missing, since I have not know 
> anything like this to be an issue in the past in other pools
> What I will do is work up a patch and submit, based on that you can 
> review it and see if is something you might be interested in.
>
> Filip
>>
>> My own opinion on needing to provide *something* here has not changed
>> -- I think we should provide a well-designed and safe capability to
>> remove abandoned connections - but wrestling with concurrency bugs
>> (e.g., the still-open DBCP-44) has made me appreciate how hard this
>> is.  Note that the AbandonedConfig and AbandonedObjectPool that
>> provide the functionality above have been deprecated and we should be
>> thinking about how to replace / remove them (so I don't think patching
>> AbandonedObjectPool is the best idea), so now is a good time to open
>> the discussion on how to provide a better solution.  Ideas /
>> suggestions welcome!
>>
>> Phil
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>


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


Re: [DBCP] Periodic clean up of abandoned connections

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Phil Steitz wrote:
> On 4/3/07, Filip Hanik - Dev Lists <de...@hanik.com> wrote:
>> Would you guys be open to a patch to implement a periodic cleanup of
>> abandoned connections
>> the following logic can cause abandoned connections to sit around
>> forever, just because you are not hitting the max in the pool.
>> Using a DB like postgres that creates a process for each connection,
>> this is a waste of resources, and the only way to trigger the abandoned
>> cleanup is to try to get to max number of connections.
>>
>>         if (config != null
>>                 && config.getRemoveAbandoned()
>>                 && (getNumIdle() < 2)
>>                 && (getNumActive() > getMaxActive() - 3) ) {
>>             removeAbandoned();
>>         }
>>
>> Let me know, and if yes, I will provide a patch
>>
>
> I agree that the above setup is not optimal.  Now is a good time to
> talk about what we want to do in this area.  The abandoned connection
> cleanup problem has sort of a rich history in [dbcp] and is a complex
> problem.  See for example:
>
> http://marc.info/?t=105633413400001&r=1&w=4
> (Start with David Graham's first post on 24-Jun-2003)
I actually wrote my own connection pool in 1999 that has abandoned 
support and this has never been a problem.
I started reading the thread, and forgive my ignorance, the thread is 
way too long for me to churn through.
The solution to a problem like this is quite simple
1. When connections are marked abandoned, the wrapper connection is 
simply marked closed, and the underlying JDBC connection is closed or 
returned to the pool with a new wrapper.
2. When applications are accessing their "java.sql.Connection" method, 
it simply throws SQLException("Connection closed-abandoned");
There is 0 concurrency issues.
I will look into the DBCP code to see if there are any logistics issue 
specifically to DBCP, but in my implementation it was a piece of cake.
<5 seconds later>
Ok, so it looks like the DBCP is not using any "wrapper" connection, 
hence everyone worries about the "connection" being references all over 
the placed.
Well, piece of cake, add a wrapper using a reflection.proxy, and the 
mystery is solved.

Furthermore, in order to save threads, the generic pool can have a 
public void tasks(){} method, that the a thread invokes.
In the GenericPool the tasks() would look like this
public void tasks() {
 if (_evictor!=null) evictor.run();//of course evictor-runnable is 
modified to not sleep or delay
}

The abandoned pool would then override it like this
public void tasks() {
  super.tasks();
  if ( getActiveCount()>0) removeAbandoned();
}

Is there something apparent I am missing, since I have not know anything 
like this to be an issue in the past in other pools
What I will do is work up a patch and submit, based on that you can 
review it and see if is something you might be interested in.

Filip
>
> My own opinion on needing to provide *something* here has not changed
> -- I think we should provide a well-designed and safe capability to
> remove abandoned connections - but wrestling with concurrency bugs
> (e.g., the still-open DBCP-44) has made me appreciate how hard this
> is.  Note that the AbandonedConfig and AbandonedObjectPool that
> provide the functionality above have been deprecated and we should be
> thinking about how to replace / remove them (so I don't think patching
> AbandonedObjectPool is the best idea), so now is a good time to open
> the discussion on how to provide a better solution.  Ideas /
> suggestions welcome!
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>


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


Re: [DBCP] Periodic clean up of abandoned connections

Posted by Phil Steitz <ph...@gmail.com>.
On 4/3/07, Filip Hanik - Dev Lists <de...@hanik.com> wrote:
> Would you guys be open to a patch to implement a periodic cleanup of
> abandoned connections
> the following logic can cause abandoned connections to sit around
> forever, just because you are not hitting the max in the pool.
> Using a DB like postgres that creates a process for each connection,
> this is a waste of resources, and the only way to trigger the abandoned
> cleanup is to try to get to max number of connections.
>
>         if (config != null
>                 && config.getRemoveAbandoned()
>                 && (getNumIdle() < 2)
>                 && (getNumActive() > getMaxActive() - 3) ) {
>             removeAbandoned();
>         }
>
> Let me know, and if yes, I will provide a patch
>

I agree that the above setup is not optimal.  Now is a good time to
talk about what we want to do in this area.  The abandoned connection
cleanup problem has sort of a rich history in [dbcp] and is a complex
problem.  See for example:

http://marc.info/?t=105633413400001&r=1&w=4
(Start with David Graham's first post on 24-Jun-2003)

My own opinion on needing to provide *something* here has not changed
-- I think we should provide a well-designed and safe capability to
remove abandoned connections - but wrestling with concurrency bugs
(e.g., the still-open DBCP-44) has made me appreciate how hard this
is.  Note that the AbandonedConfig and AbandonedObjectPool that
provide the functionality above have been deprecated and we should be
thinking about how to replace / remove them (so I don't think patching
AbandonedObjectPool is the best idea), so now is a good time to open
the discussion on how to provide a better solution.  Ideas /
suggestions welcome!

Phil

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