[
https://issues.apache.org/jira/browse/DERBY-4343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12740453#action_12740453
]
Kristian Waagan commented on DERBY-4343:
----------------------------------------
I believe I understand why the assert happens.
When obtaining the pooled connection for the third time, the variable
isolation_ is reset by the following code in Connection.completeReset:
if (closeStatementsOnClose) {
// NOTE: This is to match previous behavior.
// Investigate and check if it is really necessary.
this.isolation_ = TRANSACTION_UNKNOWN;
java.util.Set keySet = openStatements_.keySet();
for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
Object o = i.next();
((Statement) o).reset(closeStatementsOnClose);
}
} else {
When setTransactionIsolation is called, and actually executed, the server
doesn't send any piggybacked update because the level hasn't changed. This
means the isolation_ will remain as UNKNOWN until getTransactionIsolation is
called or a different statement causing a change of the isolation level is
executed.
DERBY-4314 causes the assert to never be reached by (in this case) querying the
server about the isolation level and then short-circuiting
setTransactionIsolation:
+ // Derby-4314 fix, the client driver should acted like embedded
+ if (level == getTransactionIsolationX())
+ return;
For completeness, removing the return in this case would also make the assert
pass, since getTransactionIsolationX would have updated the value of isolation_.
My take on this assert is that it is doesn't have any consequences "in the real
world". Next time the user asks for the isolation level, it will be fetched
from the server and a correct value will be returned.
I'm +1 for patch DERBY-4314-5.diff, but I'm not yet confident resetting the
value to READ_COMMITTED is the best solution (as of today, I think it is a
correct change though).
The patch won't improve the performance, it will just move the round-trip. In
some cases I believe it will make it worse for the initial
setTransactionIsolation call, depending on whether the value being set is
READ_COMMITTED or not. If it is, we still get one round-trip (check +
short-circuit), but if it isn't we get two (check + change).
Note that the situation is slightly different when the client side JDBC
statement cache is enabled.
This means that for better performance, we need additional changes.
Not thinking ahead, the simplest change would be to assume that the default
isolation level will be READ_COMMITTED for ever. If this turns out to be false
in the future, we may get out of it using version checks.
A better, but more demanding solution, would be to obtain the default isolation
level at initial connection time, for instance using the existing piggybacking
feature. This still requires a version check in the server, but gives us more
flexibility [1].
On top of this we have the XA scenario, which I understand requires some extra
care to avoid returning stale/incorrect values.
Thoughts?
[1] I'm thinking about optimizing this situation in Connection.completeReset:
} else {
// Must reset transaction isolation level if it has been changed.
if (isolation_ != Connection.TRANSACTION_READ_COMMITTED) {
// This might not fare well with connection pools, if it has
// been configured to deliver connection with a different
// isolation level, i.e. it has to set the isolation level again
// when it returns connection to client.
// TODO: Investigate optimization options.
setTransactionIsolationX(Connection.TRANSACTION_READ_COMMITTED);
}
With the described approach, the default level could be configured in the
connection url with an attribute. I do not know if this is allowed by the
relevant standards, or if it is something we ever want to do.
> ASSERT FAILED calling setTransactionIsolation checking isolation_ == level
> on pooled connection
> --------------------------------------------------------------------------------------------------
>
> Key: DERBY-4343
> URL: https://issues.apache.org/jira/browse/DERBY-4343
> Project: Derby
> Issue Type: Bug
> Components: JDBC, Network Client
> Affects Versions: 10.6.0.0
> Reporter: Kathey Marsden
> Attachments: TestConnReuse.java
>
>
> For DERBY-4314, I thought I would do a little testing to understand the
> server round trips in various scenarios for pooled connections. So I wrote
> the small attached program ConnReuse.java and hit this assertion in client:
> Exception in thread "main"
> org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED
> at
> org.apache.derby.shared.common.sanity.SanityManager.ASSERT(SanityManager.java:98)
> at
> org.apache.derby.client.am.Connection.setTransactionIsolationX(Connection.java:987)
> at
> org.apache.derby.client.am.Connection.setTransactionIsolation(Connection.java:915)
> at
> org.apache.derby.client.am.LogicalConnection.setTransactionIsolation(LogicalConnection.java:253)
> at TestConnReuse.main(TestConnReuse.java:32)
> ---------------
> To run the program on trunk:
> java org.apache.derby.drda.NetworkServerControl start
> java TestConnReuse.
> This needs more investigation, but I thought I would go ahead and log the
> bug. I tried this only on trunk. I have not yet
> - Tried it on the branches.
> - Tried it with Lily's DERBY-4314 patch.
> - Tried it with embedded.
> - Tried actually doing something with the prior logical connection which
> might be related.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
