[jira] [Comment Edited] (POOL-282) CLONE - Support AbandonedConfig in SharedPoolDataSource

["Phil Steitz (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/POOL-282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17531339#comment-17531339 ] 

Phil Steitz edited comment on POOL-282 at 5/3/22 6:08 PM:
----------------------------------------------------------

[~ashok2ashok] 

First, many thanks for stepping up to the challenge here.  Here are some comments:
 # I don't think we need to support different abandoned configs for different keys.  That would be kind of messy and I can't immediately think of scenarios where different configs would be desired.  Does your use case require this?  Assuming no, I would stick with just using the AbandonedConfig class as is.
 # The per key considerations come in when you actually do the cleanup.  It seems reasonable to me to generally follow the pattern of what GOP does, except that you add the key as a parameter to removeAbandoned, so it is ``removeAbandoned(ac, key)`` and it does what the signature suggests - just goes after abandoned instances under the given key.  So borrowObject does the same kind of test that GOP does and if the pool is running low for the given key, it does the cleanup for that key. 
 # Maintenance is kind of interesting.  The natural thing to do would be to iterate all keys and call ``removeAbandoned(ac, key)`` for all of them.  For pools with lots of keys, that might cause performance problems.  I can see three options here: 0) don't worry about it 1) hitchhike on the iteration sequence in the loop over numTests in evict and just hit the keys that get visited 2) introduce a separate config that controls how many keys get cleaned up per eviction run and maintain a separate keys iterator to cycle through them.  Could be I am needlessly worrying here and 0) is fine.  The whole thing can be turned off by setting removeAbandonedOnMaintenance to false so I think it is probably OK to start with the simple hit them all impl.


was (Author: psteitz):
[~ashok2ashok] 

First, many thanks for stepping up to the challenge here.  Here are some comments:
 # I don't think we need to support different abandoned configs for different keys.  That would be kind of messy and I can't immediately think of scenarios where different configs would be desired.  Does your use case require this?  Assuming no, I would stick with just using the AbandonedConfig class as is.
 # The per key considerations come in when you actually do the cleanup.  It seems reasonable to me to generally follow the pattern of what GOP does, except that you add the key as a parameter to removeAbandoned, so it is `removeAbandoned(ac, key)` and it does what the signature suggests - just goes after abandoned instances under the given key.  So borrowObject does the same kind of test that GOP does and if the pool is running low for the given key, it does the cleanup for that key. 
 # Maintenance is kind of interesting.  The natural thing to do would be to iterate all keys and call `removeAbandoned(ac, key)` for all of them.  For pools with lots of keys, that might cause performance problems.  I can see three options here: 0) don't worry about it 1) hitchhike on the iteration sequence in the loop over numTests in evict and just hit the keys that get visited 2) introduce a separate config that controls how many keys get cleaned up per eviction run and maintain a separate keys iterator to cycle through them.  Could be I am needlessly worrying here and 0) is fine.  The whole thing can be turned off by setting removeAbandonedOnMaintenance to false so I think it is probably OK to start with the simple hit them all impl.

> CLONE - Support AbandonedConfig in SharedPoolDataSource
> -------------------------------------------------------
>
>                 Key: POOL-282
>                 URL: https://issues.apache.org/jira/browse/POOL-282
>             Project: Commons Pool
>          Issue Type: Improvement
>    Affects Versions: 2.0, 2.1, 2.2, 2.3
>         Environment: Linux and Java 1.6
>            Reporter: Michael Kerr
>            Priority: Minor
>              Labels: features
>
> BasicDataSource has support for AbandonedConfig
> SharedPoolDataSource does not currently support reclaiming abandoned connections.
> It would be helpful if the user can extend the underlying connection pool to add features like abandoned connection management.  This is possible with BasicDataSource.
> Currently the only solution is to extend classes or replace classes in the Apache package namespace.
> Thanks for looking,
> Michael



--
This message was sent by Atlassian Jira
(v8.20.7#820007)