db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kristian Waagan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-3326) Introduce a caching logical connection and logical prepared statement in the client driver
Date Thu, 14 Feb 2008 15:23:08 GMT

    [ https://issues.apache.org/jira/browse/DERBY-3326?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12568959#action_12568959
] 

Kristian Waagan commented on DERBY-3326:
----------------------------------------

When working on this patch, I found a possible problematic area with
regards to synchronization. The problem is that a physical statement can
be taken out of the statement cache before it has been reset for reuse.
Calls will still be synchronized in the physical statement, but I think
there is a possibilty for nasty side effects due to ordering.
I have just introduced the method 'resetForReuse' for now, it does not
yet exists. It will/would involve actions like closing result sets.

Current situation, in a new file/method LogicalStatementEntity.close:
    // Try to insert the statement into the cache.
    if (cache.cacheStatement(stmtKey, temporaryPsRef)) {
        temporaryPsRef.resetForReuse();
    } else {
        // Statement was already in the cache, so we can close it.
        temporaryPsRef.close();
    }

There is a possibility for another thread to grab the physical statement
(named 'temporaryPsRef' above) just after it has been inserted.
There are several solutions, I describe some of the below. They all have
pros and cons...

* Solution #1: Do it unconditionally before insertion.
Change code to something like this:
    temporaryPsRef.resetForReuse();
    // Try to insert the statement into the cache.
    if (!cache.cacheStatement(stmtKey, temporaryPsRef)) {
        // Statement was already in the cache, so we can close it.
        temporaryPsRef.close();
    }

  - We end up doing some unnecessary work if stmt is discarded
    (this probably means an extra round-trip, but should rarely happen)
  - Harder to use the cache correctly?
  + No changes to statement cache, simple.

* Solution #2: Let the cache to the work.
One could add another interace, say 'ReusableStatement', with two
methods: discard() and resetForReuse(). Both would throw SQLException.
One would simply call cache.cacheStatement(stmtKey, temporaryPsRef), which
could do something like this while holding synchronization on the cache:
    final boolean alreadyCached = this.statements.containsKey(statementKey);
    if (!alreadyCached) {
        ps.resetForReuse();
        statements.put(statementKey, ps);
    } else {
        ps.discard();
    }
    return !alreadyCached;

One could also imagine making the cache more generic, by introducing say
'ReusableJDBCObject' and rename the cache methods etc.

  + encapsulation
  + ease of use
  - yet another interface
  - cache methods throw exceptions

* Solution #3: Introduce a synchronization object to the cache
Partially as done other places in Derby, introducing a synchronization
object for doing composite actions. This would typically be the
connection instance. With regards to solution #1, the code would change
to the code below and the cache would have to synchronize on the same
object internally:
    synchronized (cache.getSynchronizationObject()) {
        // Try to insert the statement into the cache.
        if (cache.cacheStatement(stmtKey, temporaryPsRef)) {
            temporaryPsRef.resetForReuse();
        } else {
            // Statement was already in the cache, so we can close it.
            temporaryPsRef.close();
        }
    }

  + flexibility???
  - error prone
  - cache can be locked down by clients

One could also introduce some other variation of the above, but I was
in general hoping to avoid having to synchronize too much in the logical
entities, since the associated physical entity will synchronize as well
and the reference to the physical entity will be synchronized. The
common forwarding pattern I have intended to use is:
    getPhys[CP]s().<methodName>(<args>);
The get methods will throw an exception if the logical entity has been
closed, and the get methods and the close method will synchronize on the
same object (currently this is the LogicalStatementEntity).

Have I forgotten anything essential?
Other pieces of advice anyone want to share with me?


For my current patch I will go for solution #1 due to its simplicity,
but will consider changing it later based on feedback and further
investigation.

I expect to post patches for DERBY-3326, DERBY-3328 and DERBY-3329
tomorrow. What's left for sure then, is to integrate DERBY-3192, which
will happen under the DERBY-3326 issue.
These patches might need more work, but it should then be possible to
apply a set of patches and test the feature from trunk.

> Introduce a caching logical connection and logical prepared statement in the client driver
> ------------------------------------------------------------------------------------------
>
>                 Key: DERBY-3326
>                 URL: https://issues.apache.org/jira/browse/DERBY-3326
>             Project: Derby
>          Issue Type: Sub-task
>          Components: JDBC, Network Client
>    Affects Versions: 10.4.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>             Fix For: 10.4.0.0
>
>         Attachments: derby-3326-1a_cpds_testing_preparation.diff, derby-3326-1a_cpds_testing_preparation.stat,
derby-3326-1b_cpds_testing_preparation.diff, derby-3326-2a-method_rename.diff
>
>
> A logical connection with statement caching capabilities is required to support JDBC
prepared statement pooling. Further, a logical prepared statement object is also needed.
> The logical prepared statements will be generated by the logical connection's 'prepareStatement'-method.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message