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 2009/11/01 17:12:11 UTC
Re: [dbcp] Callable statement pooling (DBCP-204)
Phil Steitz wrote:
> I agree that DBCP should support callable statement pooling and the
> patch attached to DBCP-204 is a reasonable way to do it. What I am
> concerned about is that adding callable statements to the prepared
> statement pool with the current implementation may break some
> applications by causing them to go beyond MaxPreparedStatements.
> Unfortunately, the current behavior is to throw SQLException on
> prepareStatement when this happens. I see four options.
>
> 0) Damn the torpedoes - apply patch as is
Agree - too risky.
> 1) Change the prepared statement pool to behave as an LRU cache
This is looks like the best way to go to me,
> 2) Stick with KeyedObjectPool implementation, but create and do not
> pool new statements when the pool is exhausted
I can think of scenarios where this could negate the benefit of the pooling in
the first place. 1) seems the better choice.
> 3) Leave implementation as is but add a new KeyedObjectPool for
> callable statements
Strikes me as adding unnecessary complexity.
> I think 1) is the best option, but it is a significant change in
> behavior, so should probably wait until 2.0
I can see where you are coming from but how many folks would actually complain
that a SQLException wasn't thrown if their code opened too many statements? If
we can see any scenarios where this would cause problems, then lets wait for
2.0. If we can't (and I can't right now), I'd vote for 1.3.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [dbcp] Callable statement pooling (DBCP-204)
Posted by Phil Steitz <ph...@gmail.com>.
Mark Thomas wrote:
> Phil Steitz wrote:
>> OK, I am now feeling stupid.
> You and me both.
>
>> GKOP already has LRU-cache like
>> behavior. (See clearOldest test in allocate()). Given compat
>> issues, I am inclined to move forward with 0).
>
> +1
>
> We'll also need pool 1.5.4 as well to fix an edge case where the pool
> could lock up.
Will get RCs for both this weekend. Sorry for delay.
Phil
>
> Mark
>
>
>
>
> ---------------------------------------------------------------------
> 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: [dbcp] Callable statement pooling (DBCP-204)
Posted by Mark Thomas <ma...@apache.org>.
Phil Steitz wrote:
> Mark Thomas wrote:
>> Phil Steitz wrote:
>>> OK, I am now feeling stupid.
>> You and me both.
>>
>>> GKOP already has LRU-cache like
>>> behavior. (See clearOldest test in allocate()). Given compat
>>> issues, I am inclined to move forward with 0).
>> +1
>>
>> We'll also need pool 1.5.4 as well to fix an edge case where the pool
>> could lock up.
>>
>> Mark
>>
>
> One question. The patch introduces a statement type flag as part of
> the key to distinguish prepared statements from callable statements.
> I am not sure this is necessary. Can anyone imagine a scenario
> where the same exact sql is used in prepareStatement and
> prepareCall? Are there drivers that even allow this ("call" in
> prepareStatement sql, or no "call" in prepareCall) If not, we do not
> need the additional flag.
As CallableStatement extends PreparedStatement there may be some edge
cases where the same sql could be used. It isn't something I would do
but I can how it might work with some drivers (I haven't tested any).
Given that the sql would normally be very different, I don't see the
harm including the statement type in the key apart from a small
inefficiency in key construction but I don't see that as a major issue.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [dbcp] Callable statement pooling (DBCP-204)
Posted by Phil Steitz <ph...@gmail.com>.
Mark Thomas wrote:
> Phil Steitz wrote:
>> OK, I am now feeling stupid.
> You and me both.
>
>> GKOP already has LRU-cache like
>> behavior. (See clearOldest test in allocate()). Given compat
>> issues, I am inclined to move forward with 0).
>
> +1
>
> We'll also need pool 1.5.4 as well to fix an edge case where the pool
> could lock up.
>
> Mark
>
One question. The patch introduces a statement type flag as part of
the key to distinguish prepared statements from callable statements.
I am not sure this is necessary. Can anyone imagine a scenario
where the same exact sql is used in prepareStatement and
prepareCall? Are there drivers that even allow this ("call" in
prepareStatement sql, or no "call" in prepareCall) If not, we do not
need the additional flag.
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: [dbcp] Callable statement pooling (DBCP-204)
Posted by Mark Thomas <ma...@apache.org>.
Phil Steitz wrote:
> OK, I am now feeling stupid.
You and me both.
> GKOP already has LRU-cache like
> behavior. (See clearOldest test in allocate()). Given compat
> issues, I am inclined to move forward with 0).
+1
We'll also need pool 1.5.4 as well to fix an edge case where the pool
could lock up.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [dbcp] Callable statement pooling (DBCP-204)
Posted by Phil Steitz <ph...@gmail.com>.
Phil Steitz wrote:
> Phil Steitz wrote:
>> Mark Thomas wrote:
>>> Phil Steitz wrote:
>>>> I agree that DBCP should support callable statement pooling and the
>>>> patch attached to DBCP-204 is a reasonable way to do it. What I am
>>>> concerned about is that adding callable statements to the prepared
>>>> statement pool with the current implementation may break some
>>>> applications by causing them to go beyond MaxPreparedStatements.
>>>> Unfortunately, the current behavior is to throw SQLException on
>>>> prepareStatement when this happens. I see four options.
>>>>
>>>> 0) Damn the torpedoes - apply patch as is
>>> Agree - too risky.
>>>
>>>> 1) Change the prepared statement pool to behave as an LRU cache
>>> This is looks like the best way to go to me,
>>>
>>>> 2) Stick with KeyedObjectPool implementation, but create and do not
>>>> pool new statements when the pool is exhausted
>>> I can think of scenarios where this could negate the benefit of the pooling in
>>> the first place. 1) seems the better choice.
>> +1
>>>> 3) Leave implementation as is but add a new KeyedObjectPool for
>>>> callable statements
>>> Strikes me as adding unnecessary complexity.
>> +1
>>>> I think 1) is the best option, but it is a significant change in
>>>> behavior, so should probably wait until 2.0
>>> I can see where you are coming from but how many folks would actually complain
>>> that a SQLException wasn't thrown if their code opened too many statements? If
>>> we can see any scenarios where this would cause problems, then lets wait for
>>> 2.0. If we can't (and I can't right now), I'd vote for 1.3.
>> I agree with you. I had never run into this and was frankly
>> surprised when I realized that the current impl works this way. I
>> guess the documentation for maxOpenPreparedStatements is vague
>> enough (no mention of SQLExceptions on pool exhaustion) that we
>> could change the behavior now (in 1.3).
>>
>> I will create a patch using a LinkedHashMap for the cache and attach
>> it to DBCP-204.
>
> Crap. Just realized that _pstmtPool is a &%^$# *protected* field in
> PoolingConnection. I am inclined to ignore this compat break.
> Interested in what others think.
OK, I am now feeling stupid. GKOP already has LRU-cache like
behavior. (See clearOldest test in allocate()). Given compat
issues, I am inclined to move forward with 0).
Phil
>
> Phil
>> Phil
>>
>>> Mark
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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: [dbcp] Callable statement pooling (DBCP-204)
Posted by Phil Steitz <ph...@gmail.com>.
Phil Steitz wrote:
> Mark Thomas wrote:
>> Phil Steitz wrote:
>>> I agree that DBCP should support callable statement pooling and the
>>> patch attached to DBCP-204 is a reasonable way to do it. What I am
>>> concerned about is that adding callable statements to the prepared
>>> statement pool with the current implementation may break some
>>> applications by causing them to go beyond MaxPreparedStatements.
>>> Unfortunately, the current behavior is to throw SQLException on
>>> prepareStatement when this happens. I see four options.
>>>
>>> 0) Damn the torpedoes - apply patch as is
>> Agree - too risky.
>>
>>> 1) Change the prepared statement pool to behave as an LRU cache
>> This is looks like the best way to go to me,
>>
>>> 2) Stick with KeyedObjectPool implementation, but create and do not
>>> pool new statements when the pool is exhausted
>> I can think of scenarios where this could negate the benefit of the pooling in
>> the first place. 1) seems the better choice.
> +1
>>> 3) Leave implementation as is but add a new KeyedObjectPool for
>>> callable statements
>> Strikes me as adding unnecessary complexity.
> +1
>>> I think 1) is the best option, but it is a significant change in
>>> behavior, so should probably wait until 2.0
>> I can see where you are coming from but how many folks would actually complain
>> that a SQLException wasn't thrown if their code opened too many statements? If
>> we can see any scenarios where this would cause problems, then lets wait for
>> 2.0. If we can't (and I can't right now), I'd vote for 1.3.
>
> I agree with you. I had never run into this and was frankly
> surprised when I realized that the current impl works this way. I
> guess the documentation for maxOpenPreparedStatements is vague
> enough (no mention of SQLExceptions on pool exhaustion) that we
> could change the behavior now (in 1.3).
>
> I will create a patch using a LinkedHashMap for the cache and attach
> it to DBCP-204.
Crap. Just realized that _pstmtPool is a &%^$# *protected* field in
PoolingConnection. I am inclined to ignore this compat break.
Interested in what others think.
Phil
>
> Phil
>
>> Mark
>>
>>
>> ---------------------------------------------------------------------
>> 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: [dbcp] Callable statement pooling (DBCP-204)
Posted by Phil Steitz <ph...@gmail.com>.
Mark Thomas wrote:
> Phil Steitz wrote:
>> I agree that DBCP should support callable statement pooling and the
>> patch attached to DBCP-204 is a reasonable way to do it. What I am
>> concerned about is that adding callable statements to the prepared
>> statement pool with the current implementation may break some
>> applications by causing them to go beyond MaxPreparedStatements.
>> Unfortunately, the current behavior is to throw SQLException on
>> prepareStatement when this happens. I see four options.
>>
>> 0) Damn the torpedoes - apply patch as is
> Agree - too risky.
>
>> 1) Change the prepared statement pool to behave as an LRU cache
> This is looks like the best way to go to me,
>
>> 2) Stick with KeyedObjectPool implementation, but create and do not
>> pool new statements when the pool is exhausted
> I can think of scenarios where this could negate the benefit of the pooling in
> the first place. 1) seems the better choice.
+1
>
>> 3) Leave implementation as is but add a new KeyedObjectPool for
>> callable statements
> Strikes me as adding unnecessary complexity.
+1
>
>> I think 1) is the best option, but it is a significant change in
>> behavior, so should probably wait until 2.0
> I can see where you are coming from but how many folks would actually complain
> that a SQLException wasn't thrown if their code opened too many statements? If
> we can see any scenarios where this would cause problems, then lets wait for
> 2.0. If we can't (and I can't right now), I'd vote for 1.3.
I agree with you. I had never run into this and was frankly
surprised when I realized that the current impl works this way. I
guess the documentation for maxOpenPreparedStatements is vague
enough (no mention of SQLExceptions on pool exhaustion) that we
could change the behavior now (in 1.3).
I will create a patch using a LinkedHashMap for the cache and attach
it to DBCP-204.
Phil
>
> Mark
>
>
> ---------------------------------------------------------------------
> 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