db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel John Debrunner (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-3192) Cache session data in the client driver
Date Tue, 18 Dec 2007 18:31:43 GMT

    [ https://issues.apache.org/jira/browse/DERBY-3192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12552846
] 

Daniel John Debrunner commented on DERBY-3192:
----------------------------------------------

> In retrospect, there are some unforeseen complications that mandate more explanation,
but I'm guessing that they would not have been part of an initial writeup either... 

It's not an initial writeup I was looking for, but some summary of the actual change being
made.

The description is this bug has a couple of places where it says "I think ....", so that doesn't
really help a reviewer of the code. Did the implementor solve this issues using the ideas
that followed the "I think", or was it handled some other way?

It's also not clear from the description that the mechanism to be used is the same as setQueryTimeout,
it's not mentioned until the last comment.

I personally just don't know how to review a 1552 line patch without a good overview of is
being attempted and how it is being attempted, I'm then just reading code that obviously does
X without knowing if X was what is intended or not.

These comments in this issue worry me:
  - "the caching will increase the size of a request from the client by about 30 bytes" -
is this every request, seems too much to me?
  - "When executing a query with a scrollable result  ..." - why is scrollable result set
called out here, at a high level I can't see why the scrollability of a result set would affect
caching of session state?

Of course no-one has to provide a writeup, as long as a committer has confidence in a patch
it can be committed, but producing a writeup has the muilti-purpose of adding to the community
knowledge, potentially improving the quality of the code & reviews and often helps the
implementor ensure they fully understand the changes they are making



> Cache session data in the client driver
> ---------------------------------------
>
>                 Key: DERBY-3192
>                 URL: https://issues.apache.org/jira/browse/DERBY-3192
>             Project: Derby
>          Issue Type: Improvement
>          Components: JDBC, Network Client, Network Server, Performance, SQL
>    Affects Versions: 10.3.1.4
>            Reporter: Dyre Tjeldvoll
>            Assignee: Dyre Tjeldvoll
>         Attachments: derby-3192-test.fup1.diff, derby-3192-test.fup2.diff, derby-3192-test.v1.diff,
derby-3192-test.v1.stat, derby-3192.prelim1.diff
>
>
> The reason for doing this is to avoid a rather
> substantial performance hit observed when the client driver is used
> together with an appserver that uses connection pooling. There are two
> problems:
> 1) The connection pool will compare the isolation level it has
> stored for the connection with the value returned from
> Connection.getTransactionIsolation() each and every time someone
> requests a new connection from the pool.
> 2) The users of the connection pool (ab)use it to avoid having to keep
> track of their current connection. So each time a query needs to be
> executed a call to the connection pool's getConnection() method is
> made. Getting a connection from the connection pool like this also
> means that a new PreparedStatement must be prepared each time.
> The net result is that each query results in the following sequence:
> getConnection()
> getTransactionIsolation() --> roundtrip + lookup in server's statement cache
> prepareStatment()         --> roundtrip + lookup in server's statement cache
> executeQuery()            --> roundtrip
> Arguably this is a "user error" but when suggesting this I'm kindly
> informed that this works "just fine" with other datbases (such as
> PostgreSQL and ORACLE). 
> The reason why it works is that these databases do statement caching
> in the driver. I've tried to implement a very (too) simple statement
> cache in Derby's client driver and to re-enable caching of the
> isolation level (see
> https://issues.apache.org/jira/browse/DERBY-1148). With these changes
> I observe a marked performance improvement when running with appserver
> load. 
> A proper statment cache cannot be implemented without knowing what the
> current schema is. If the current schema has changed since the
> statement was prepared, it is no longer valid and must be evicted from
> the cache.
> The problem with caching both the isolation level and the current schema in
> the driver is that both can change on the server without the client
> detecting it (through SQL and XA and possibly stored procedures).
> I think this problem can be overcome if we piggy-back the information we would 
> like to cache on messages going back to the client. This can be done by
> utilizing the EXCSQLSET DRDA command. According to the DRDA spec (v4, volume 3, 
> page 359-360) it is possible to add one or more SQLSTT objects after SQLCARD in the reply,
> I think it would be possible to cache additional session information when this becomes
relevant.  It
> would also be possible to use EXCSQLSET to batch session state changes
> going from the client to the server.

-- 
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