|
If anyone knows of the history behind state information related to isolation level and readonly mode in BrokeredConnection, please share them.
I have been on vacation for last 3 weeks so didn't get a chance to followup further on this issue. Does anyone have any comments on why we keep the state information about isolation level and readonly?
State information is kept in BrokeredConnection for this reason (comment in BrokeredConnection)
/** Maintain state as seen by this Connection handle, not the state of the underlying Connection it is attached to. */ To expand on that, an application getting a Connection C from an XADataSource/XAConnection can use C in local mode or in global mode. The application can set C's state with various setXXX calls, and thus the applcation has expectation of what the state is. When C is connected to a local transaction, the state of the connection needs to match any state set by the application. When C is attached an existing global transaction the C's state needs to match that of the exsiting transaction, not the state as expected by the application. Then when reverting to local mode, the state needs to be reset as the application expects it. Here's a simple example. c.setReadOnly(true); // do some local work // now attach to an existing global transaction c.isReadOnly(); // returns false as the transaction has included modifications // now detach from the global transaction // This is the critical point c..isReadOnly(); // needs to return true because that's what the application is expecting Thus the BrokeredConnection maintains the state that it needs to set on the new local underlying EmbedConnection object to ensure its state is in sync with the application's expectation. This is also because the Derby embedded implementation always creates a new EmbedConnection object when switching from global to local. Attaching a patch for this JIRA entry. Following is some information on the patch.
When a connection object is obtained through XADataSource, it can be part of a local transaction or it can be attached to a global transaction. The state of the connection object can be different depending on whether it is used in local transaction or global transaction. Among other state information, isolation level is saved as part of the connection object's state information. This isolation level state is set correctly when isolation level is set using JDBC api. But it gets out of sync when SQL is used to set the isolation level state. In order to fix this, I have added a flag in GenericLanguageConnectionContext which will get set to true anytime isolation level is set using JDBC/SQL api. And this flag is later used to keep the isolation level state information uptodate in BrokeredConnection. The classes changed by this fix are as follows. svn stat M java\engine\org\apache\derby\impl\sql\conn\GenericLanguageConnectionContext.java M java\engine\org\apache\derby\iapi\sql\conn\LanguageConnectionContext.java M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnection.java M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnectionControl.java M java\engine\org\apache\derby\jdbc\EmbedPooledConnection.java M java\engine\org\apache\derby\jdbc\EmbedXAConnection.java M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\checkDataSource.java M java\testing\org\apache\derbyTesting\functionTests\master\checkDataSource.out M java\testing\org\apache\derbyTesting\functionTests\master\checkDataSource30.out I'll look into committing the patch
A few questions on the patch:
1) In BrokeredConnection.setState() the method getIsolationUptoDate() is called. This new method updates the BrokeredConnection state according to the state of the underlying connection, but the purpose and description of the setState() method is the opposite. Thus this method call looks out pf place. Additional comments indicating why it is here would be useful. 2) The new field isolationLevelSetUsingSQLorJDBC in GenericLanguageConnectionContext seems to have similar functionality to the existing field isolationLevelExplicitlySet. Are two fields really required? Usually multiple similar state fields is a recipe for bugs. 3) The number of new methods to acheive this functionality seems high, I wonder if there is a simpler approach, it would be easier to tell if you could expand on your comment 'And this flag is later used to keep the isolation level state information uptodate in BrokeredConnection.' Could you comment on exactly when this flag is used to reset the state? As for question 2), regarding the existing field isolationLevelExplicitlySet, I see that it gets set to true when the isolation level is set using JDBC or SQL. But I don't see it getting reset to false. Maybe someone who has worked on this flag can share the purpose of this flag. I noticed that this flag gets used later in GenericStatement.prepMinion to set the isolation level of the CompilerContext.
What I need is some kind of flag which will get set to true if isolation has been set using JDBC or SQL in a given transaction. When entering/exiting a global transaction later, I want to see if the flag is set to true and if so, set the isolation state in BrokeredConnection to that of EmbedConnection (since the isolation state in BrokeredConnection can be incorrect if SQL was used to set the isolation level). After this state update in BrokeredConnection, I want to reset the flag back to false. I don't think I can use isolationLevelExplicitlySet for this functionality but more information will clarify this more. BACKGROUND:
For JCC connections to Network Server, when the user sets the isolaton with setTransactionIsolation, the isolation level is encoded in each subsequent statement's package name rather than being set for the connection. see DRDAStatement.setPkgnamcsn(String pkgnamcsn) for the package name format. Network server calls setPrepareIsolation(level) to set prepareIsolationLevel when this occurs and the statement gets prepared with that isolation level. As I recall, if the isolation level is set with an SQL statement it basically overrides the prepare isolation, isolationLevelExplicitlySet gets set and the prepareIsolation is ignored. I don't think it can get reset back to false, because that would turn off the override. In the case you describe I don't quite understand why it would be desirable to set the flag to false after updating the state. If we did, wouldn't that cause trouble if we started another global transaction later? Kathey, thanks for your explanation of prepareIsolationLevel in GenericLanguageConnectionContext.java.
As for your question, following set of steps hopefully will help understand why the flag needs to be reset. Say, the user application has following code. XAConnection xac = xaDataSource.getXAConnection(); At this point, xac will be EmbedXAConnection object with real database connection EmbedConnection in it. The user application can then request a connection handle (BrokeredConnection) from EmbedXAConnection. Derby will set the state of this connection handle to that of the real database connection. Connection con = xac.getConnection(); Let's say the local transaction then sets the state using JDBC api con.setTransactionIsolation(Connection.TRANSACTION_READ_UNCOMMITTED); This call will change the isolation level of the real database connection to the new value and the state of the connection handle also gets set correctly inside BrokeredConnection.setTransactionIsolation method. But if the local transaction sets the state using SQL BrokeredStatement.executeUpdate("set current isolation = RR"); real database connection's isolation level gets set to new value but the state information in BrokeredConnection doesn't. Now if the user application says XAResource.start(xid, XAResource.TMNOFLAGS); Before starting a global transaction, EmbedXAConnection's start method checks if there is already a real database connection and a handle to it. If yes, it calls connection handle's setState(true). (The current Derby code without my changes) sets the state of the real database connection object to the handle's state. But handle's state is out of sync at this point because of earlier SQL set current isolation. And hence the state setting onto real connection will be incorrect. (this is the cause of Derby-421) To fix this, I have a call to connection handle's (BrokeredConnection's) getIsolationUptoDate() method prior to setting the real connection's state to connection handle's state. If the isolation was changed via JDBC/SQL, the connection handle's state will be brought to the correct value, and the flag used to keep track of isolation level change will be set to false. Similar thing needs to happen when the global transaction is ended because isolation level could have been set using SQL inside the global transaction as shown below. BrokeredStatement.executeUpdate("set current isolation = UR"); Again, this will update the real database connection's isolation to UR, but BrokeredConnection's state does not get changed. To correct the state information of BrokeredConnection, when the global transation is ended as follows XAResource.end(xid, XAResource.TMSUCCESS); EmbedXAConnection's end method checks if the BrokeredConnection is not null. If not null, it brings the isolation level upto date by calling BrokeredConnection.getIsolationUptoDate and then resets the flag. If say, the flag is not reset after fixing the BrokeredConnection's state information during the global transaction start and end times, then once isolation level is changed in a global transaction, local transaction will always take the isolation level of last global transaction that just ended. This is incorrect. Local transaction should take the isolation level of last global transaction only if the isolation level was changed in that transaction. Following is the incorrect behavior with no flag resetting: Start a local transaction. isolation level default CS Start global transaction 1 isolation level default CS Change isolation level in global transaction 1 to UR(the flag gets set to true at this point) Exit global transaction 1. Isolation level is local transaction isolation level UR Change isolation level in local transaction to RS Start global transaction 2 isolation level RS End global transaction 2 and rejoin global transaction 1 isolation level UR Exit global transaction 1. The isolation level in local transaction at this point should have been RS but it got set to UR which is the last global transaction's isolation level. This happened because the flag was not reset to false at the end of first global transaction when the isolation was set using JDBC/SQL. Kathey, I hope this is not too much information and helps answer your question. If I have missed a scenario in which we could run into problem, let me know. Also, Dan, I am making couple changes to the code and putting some more comments and will send a new review package soon(after running the tests) with explanation as to why we need so many methods. Here is another review package with more code comments and little reorganization of the code. The files changed remain the same
as the previous patch. M java\engine\org\apache\derby\impl\sql\conn\GenericLanguageConnectionContext.java M java\engine\org\apache\derby\iapi\sql\conn\LanguageConnectionContext.java M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnection.java M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnectionControl.java M java\engine\org\apache\derby\jdbc\EmbedPooledConnection.java M java\engine\org\apache\derby\jdbc\EmbedXAConnection.java M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\checkDataSource.java M java\testing\org\apache\derbyTesting\functionTests\master\checkDataSource.out M java\testing\org\apache\derbyTesting\functionTests\master\checkDataSource30.out This patch gets the isolation level state information in BrokeredConnection in sync with the Real Connection's isolation level state. This is necessary because any isolation level changes using SQL do not get to BrokeredConnection and hence BrokeredConnection can end with the incorrect isolation level state. The change adds a new flag in GenericLanguageConnectionContext, namely isolationLevelSetUsingSQLorJDBC. This flag gets set to true anytime isolation level is changed using JDBC or SQL. Later, at the start and end of a global transaction, EmbedXAConnection checks if the flag is set to true and if yes, it puts the real connection's isolation level into into BrokeredConnection and then resets the flag in GenericLanguageConnectionContext. The reason for adding methods for this flag maintenance in both BrokeredConnection and EmbedPooledConnection is that BrokeredConnection does not have access to GenericeLanguageConnectionContext which has the flag. EmbedPooledConnection does have access to GenericLanguageConnectionContext and hence the methods in BrokeredConnection have to go through EmbedPooledConnection. I have run the tests and the patch didn't cause any failures. Thanks for the explanations and improved comments on the code. I'll submit this patch but there are opportunities to reduce the code.
1) Note that the flag you've added is always required to be reset once it is read, thus the reset could be combined into the method used to read it, thus removing the two reset methods. 2) It's possible as well that the read method could instead return the isolation level needed to be stored in the BrokeredConnection (logical connection). Thus this would remove an extra method call and remove the need for the try catch block, as the method returning the isolation level would not need to declare throing any exceptions. 3) Possibly the start() and end() methods could get the isolation level (since they have a reference to the physical connection as an EmbedConnection) and pass it into BrokeredConnection, thus removing the need for the methods added to EmbedPooledConnection I merged the main codeline changes for this bug into 10.1 codeline on my machine and ran all the tests and they ran with no new failures. Here is the merge command I used on 10.1 codeline
svn merge -r 279086:279087 https://svn.apache.org/repos/asf/db/derby/code/trunk/ Can a commiter please commit this onto 10.1 codeline? This is the first time merging from main to branch for me, so let me know if a commiter would need more information to commit this. commtted this to 10.1
Date: Thu Sep 8 15:45:47 2005 New Revision: 279665 URL: http://svn.apache.org/viewcvs?rev=279665&view=rev The fix went into both 10.2 and 10.1 codeline
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Sub-task
Closed
Major
When SQL "SET CURRENT ISOLATION = newValue" is used in XA environment, outside the global transaction, it ends up calling GenericLanguageConnectionContext.setTransactionIsolation().
When JDBC api Connection.setTransactionIsolation is used, in XA environment, outside the global transaction, it calls BrokeredConnection.setTransactionIsolation(), which looks as follows
public final void setTransactionIsolation(int level) throws SQLException
{
try {
getRealConnection().setTransactionIsolation(level);
stateIsolationLevel = level;
} catch (SQLException sqle) {
notifyException(sqle);
throw sqle;
}
}
Just like in the case of sql processing, getRealConnection().setTransactionIsolation(level) ends up in GenericLanguageConnectionContext.setTransactionIsolation(). And so the behavior with JDBC api and SQL is the same so far. But with JDBC api, the additional step of setting stateIsolationLevel in BrokeredConnection to the new isolation level happens but this step is missing when using SQL. Later, when global transaction is started, in EmbedXAConnection.start(), the value of stateIsolationLevel gets used but this variable does not have updated value when coming through the SQL "SET CURRENT ISOLATION = newValue".
What I don't understand is why do we have to keep the isolation level and readonly information is BrokeredConnection.java. We need to keep autocommit and holdability information because they get changed by Derby inside the global transaction to false and CLOSE_CURSORS_ON_COMMIT and we need to restore the original values when the connection is out of the global transaction. But why do we have to keep the state information for isolation level and readonly. I think once I understand this, I will be able to make more progress on the bug.