db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kmars...@apache.org
Subject svn commit: r940620 - in /db/derby/code/trunk/java: client/org/apache/derby/client/am/ client/org/apache/derby/client/net/ drda/org/apache/derby/impl/drda/ testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/
Date Mon, 03 May 2010 21:17:04 GMT
Author: kmarsden
Date: Mon May  3 21:17:03 2010
New Revision: 940620

URL: http://svn.apache.org/viewvc?rev=940620&view=rev
Log:
DERBY-4314 With derby client setTrasactionIsolation executes and commits even if isolation
has not changed.

Contributed by Lily Wei (lilywei at yahoo dot com) and Kristian Waagan
setTransactionIsolation will no longer commit.
Piggybacking of the isolation level was implemented to avoid a performance impact.
There is specialized logic for XA Resume and Join to make sure the client isolation is in
sync with the server.



Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnectionReply.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAConnectionReply.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAResource.java
    db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/DRDAConnThread.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/SetTransactionIsolationTest.java

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java Mon May  3
21:17:03 2010
@@ -26,6 +26,7 @@ import org.apache.derby.jdbc.ClientDataS
 import org.apache.derby.shared.common.reference.SQLState;
 
 import java.sql.SQLException;
+import org.apache.derby.client.net.NetXAResource;
 import org.apache.derby.shared.common.sanity.SanityManager;
 
 public abstract class Connection implements java.sql.Connection,
@@ -104,6 +105,13 @@ public abstract class Connection impleme
      * piggy-backing.
      */
     private int isolation_ = TRANSACTION_UNKNOWN;
+    /**
+     * The default isolation level, enforced on connection resets.
+     * <p>
+     * Note that this value may be changed upon connection initialization in
+     * the future, as the server can piggy-back the isolation level.
+     */
+    private int defaultIsolation = TRANSACTION_READ_COMMITTED;
 
     /**
      * Cached copy of the schema name. Updated through piggy-backing and used
@@ -909,6 +917,21 @@ public abstract class Connection impleme
         if (agent_.loggingEnabled()) {
             agent_.logWriter_.traceEntry(this, "setTransactionIsolation", level);
         }
+        //This avoids assertion like DERBY-4343 by short-circuiting
+        //setTransactionIsolation. Before this check, for case as users
+        //obtaining the pooled connection for the third time, the variable
+        //isolation_ is reset Connection.completeReset. 
+        //Isolation_ remain as UNKNOWN until getTransactionIsolation is called
+        //or a different statement causing a change of the isolation level
+        //is executed.We might think about change the default value for Isolation_
+        //to DERBY_TRANSACTION_READ_COMMITTED. With introducing 
+        //getTransactionIsolationX and this check, assertion is never reach. 
+        //As part of DERBY-4314 fix, the client driver should act as embedded 
+        //and return here, otherwise setTransactionIsolation will commit 
+        //the transaction which is not the intention.
+        if (level == getTransactionIsolationX()) 
+	    return;
+
         try {
             // Per jdbc spec (see java.sql.Connection.close() javadoc).
             checkForClosedConnection();
@@ -1010,7 +1033,20 @@ public abstract class Connection impleme
     protected abstract boolean serverSupportsTimestampNanoseconds();
 
     public int getTransactionIsolation() throws SQLException {
-    	
+
+        if (agent_.loggingEnabled()) {
+            agent_.logWriter_.traceEntry(this, "getTransactionIsolation", isolation_);
+        }
+        try {
+            // Per jdbc spec (see java.sql.Connection.close() javadoc).
+            checkForClosedConnection();
+            return getTransactionIsolationX();
+        } catch (SqlException se) {
+            throw se.getSQLException();
+        }
+    }
+
+    public int getTransactionIsolationX() throws SQLException {
     	// Store the current auto-commit value and use it to restore 
     	// at the end of this method.
     	boolean currentAutoCommit = autoCommit_;
@@ -1019,9 +1055,6 @@ public abstract class Connection impleme
         try
         {
             checkForClosedConnection();
-            if (agent_.loggingEnabled()) {
-                agent_.logWriter_.traceExit(this, "getTransactionIsolation", isolation_);
-            }
             
             if (isolation_ != TRANSACTION_UNKNOWN) {
                 if (SanityManager.DEBUG) {
@@ -2093,6 +2126,33 @@ public abstract class Connection impleme
         isolation_ = pbIsolation;
     }
 
+    /**
+     * Sets the default isolation level of the connection upon connection
+     * initialization.
+     * <p>
+     * Note that depending on the server version, the default isolation value
+     * may not be piggy-backed on the initialization flow. In that case, the
+     * default is assumed / hardcoded to be READ_COMMITTED.
+     *
+     * @param pbIsolation isolation level as specified by
+     *      {@code java.sql.Connection}
+     */
+    public void completeInitialPiggyBackIsolation(int pbIsolation) {
+        if (SanityManager.DEBUG) {
+            SanityManager.ASSERT(
+                    pbIsolation == 
+                        java.sql.Connection.TRANSACTION_READ_UNCOMMITTED ||
+                    pbIsolation ==
+                        java.sql.Connection.TRANSACTION_READ_COMMITTED ||
+                    pbIsolation ==
+                        java.sql.Connection.TRANSACTION_REPEATABLE_READ ||
+                    pbIsolation ==
+                        java.sql.Connection.TRANSACTION_SERIALIZABLE,
+                    "Invalid isolation level value: " + pbIsolation);
+        }
+        defaultIsolation = isolation_ = pbIsolation;
+    }
+
     public void completePiggyBackSchema(String pbSchema) {
         if (SanityManager.DEBUG) {
             SanityManager.ASSERT(supportsSessionDataCaching());
@@ -2100,6 +2160,15 @@ public abstract class Connection impleme
         currentSchemaName_ = pbSchema;
     }
 
+    /**
+     * Sets the current schema upon connection initialization.
+     *
+     * @param pbSchema the schema name
+     */
+    public void completeInitialPiggyBackSchema(String pbSchema) {
+        currentSchemaName_ = pbSchema;
+    }
+
     public abstract void addSpecialRegisters(String s);
 
     // can this only be called by the PooledConnection
@@ -2149,7 +2218,8 @@ public abstract class Connection impleme
      *      pooling is enabled, and a more lightweight reset procedure is used.
      */
     protected void completeReset(boolean isDeferredReset,
-                                 boolean closeStatementsOnClose)
+                                 boolean closeStatementsOnClose,
+                                 NetXAResource xares)
             throws SqlException {
         open_ = true;
 
@@ -2160,24 +2230,24 @@ public abstract class Connection impleme
         // Iterate through the physical statements and re-enable them for reuse.
 
         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 {
-            // 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);
-            }
+        }
+        // Must reset transaction isolation level if it has been changed,
+        // except when we are doing XA and resuming/joining a global tx.
+        if (xares != null && xares.keepCurrentIsolationLevel()) {
+            // Reset the flag, do nothing else.
+            xares.setKeepCurrentIsolationLevel(false);
+        } else if (isolation_ != defaultIsolation) {
+            // This might not fare well with connection pools, if it has
+            // been configured to deliver connections 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(defaultIsolation);
         }
 
         if (!isDeferredReset && agent_.loggingEnabled()) {

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java Mon May
 3 21:17:03 2010
@@ -372,7 +372,7 @@ public class NetConnection extends org.a
 
     protected void completeReset(boolean isDeferredReset)
             throws SqlException {
-        super.completeReset(isDeferredReset, closeStatementsOnClose);
+        super.completeReset(isDeferredReset, closeStatementsOnClose, xares_);
     }
 
     public void flowConnect(String password,

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnectionReply.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnectionReply.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnectionReply.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnectionReply.java Mon
May  3 21:17:03 2010
@@ -299,6 +299,7 @@ public class NetConnectionReply extends 
         }
 
         parseACCRDBRM(netConnection);
+        parseInitialPBSD(netConnection);
         peekCP = peekCodePoint();
         if (peekCP == Reply.END_OF_SAME_ID_CHAIN) {
             return;
@@ -3314,6 +3315,41 @@ public class NetConnectionReply extends 
         }
     }
 
+     /**
+     * Parse the initial PBSD - PiggyBackedSessionData code point.
+     * <p>
+     * If sent by the server, it contains a PBSD_ISO code point followed by a
+     * byte representing the JDBC isolation level, and a PBSD_SCHEMA code point
+     * followed by the name of the current schema as an UTF-8 String.
+     *
+     * @throws org.apache.derby.client.am.DisconnectException
+     */
+    protected void parseInitialPBSD(Connection connection)
+            throws DisconnectException {
+        if (peekCodePoint() != CodePoint.PBSD) {
+            return;
+        }
+        parseLengthAndMatchCodePoint(CodePoint.PBSD);
+        int peekCP = peekCodePoint();
+        while (peekCP != END_OF_SAME_ID_CHAIN) {
+            parseLengthAndMatchCodePoint(peekCP);
+            switch (peekCP) {
+                case CodePoint.PBSD_ISO:
+                    netAgent_.netConnection_.
+                        completeInitialPiggyBackIsolation(readUnsignedByte());
+                    break;
+                case CodePoint.PBSD_SCHEMA:
+                    netAgent_.netConnection_.
+                        completeInitialPiggyBackSchema
+                            (readString(getDdmLength(), "UTF-8"));
+                    break;
+                default:
+                    parseCommonError(peekCP);
+            }
+            peekCP = peekCodePoint();
+        }
+    }
+
     /**
      * Parse a PBSD - PiggyBackedSessionData code point. Can contain one or
      * both of, a PBSD_ISO code point followed by a byte representing the jdbc

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAConnectionReply.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAConnectionReply.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAConnectionReply.java
(original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAConnectionReply.java
Mon May  3 21:17:03 2010
@@ -60,10 +60,21 @@ public class NetXAConnectionReply extend
     protected void readXaStartUnitOfWork(NetConnection conn) throws DisconnectException {
         startSameIdChainParse();
         parseSYNCCTLreply(conn);
+        // If we are joining or resuming a global transaction, we let the
+        // server set the transcation isolation state for us.
+        // Otherwise we do a normal reset.
+        NetXACallInfo callInfo =
+                conn.xares_.callInfoArray_[conn.currXACallInfoOffset_];
+        boolean keep = callInfo.xaFlags_ == XAResource.TMJOIN ||
+                callInfo.xaFlags_ == XAResource.TMRESUME;
+        conn.xares_.setKeepCurrentIsolationLevel(keep);
         endOfSameIdChainData();
     }
 
     protected int readXaEndUnitOfWork(NetConnection conn) throws DisconnectException {
+        // We have ended the XA unit of work, the next logical connection
+        // should be reset using the normal procedure.
+        conn.xares_.setKeepCurrentIsolationLevel(false);
         NetXACallInfo callInfo = conn.xares_.callInfoArray_[conn.currXACallInfoOffset_];
         int xaFlags = callInfo.xaFlags_;
 

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAResource.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAResource.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAResource.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/net/NetXAResource.java Mon May
 3 21:17:03 2010
@@ -41,6 +41,7 @@ package org.apache.derby.client.net;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.sql.SQLException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.LinkedList;
@@ -87,6 +88,7 @@ public class NetXAResource implements XA
     XAConnection xaconn_;
     org.apache.derby.client.net.NetXAConnection netXAConn_;
     org.apache.derby.client.net.NetConnection conn_;
+    private boolean keepIsolationLevel;
     int rmId_; // unique RmId generated by XAConnection
     // TODO: change to a single callInfo field (not an array)
     NetXACallInfo callInfoArray_[] =
@@ -561,6 +563,14 @@ public class NetXAResource implements XA
         return true;
     }
 
+    public void setKeepCurrentIsolationLevel(boolean flag) {
+        keepIsolationLevel = flag;
+    }
+
+    public boolean keepCurrentIsolationLevel() {
+        return keepIsolationLevel;
+    }
+
     /**
      * Start work on behalf of a transaction branch specified in xid
      *

Modified: db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/DRDAConnThread.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/DRDAConnThread.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/DRDAConnThread.java (original)
+++ db/derby/code/trunk/java/drda/org/apache/derby/impl/drda/DRDAConnThread.java Mon May 
3 21:17:03 2010
@@ -3493,6 +3493,21 @@ class DRDAConnThread extends Thread {
 								 CodePoint.TYPDEFNAM_QTDSQLASC);
 		writeTYPDEFOVR();
 		writer.endDdmAndDss ();
+
+         // Write the initial piggy-backed data, currently the isolation level
+         // and the schema name. Only write it if the client supports session
+         // data caching.
+         // Sending the session data on connection initialization was introduced
+         // in Derby 10.7.
+         if ((appRequester.getClientType() == appRequester.DNC_CLIENT) &&
+                 appRequester.greaterThanOrEqualTo(10, 7, 0)) {
+             try {
+                 writePBSD();
+             } catch (SQLException se) {
+                 server.consoleExceptionPrint(se);
+                 errorInChain(se);
+             }
+         }
 		finalizeChain();
 	}
 	

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/SetTransactionIsolationTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/SetTransactionIsolationTest.java?rev=940620&r1=940619&r2=940620&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/SetTransactionIsolationTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/SetTransactionIsolationTest.java
Mon May  3 21:17:03 2010
@@ -144,11 +144,7 @@ public class SetTransactionIsolationTest
     /**
      * setTransactionIsolation commits?
      */
-    public void testSetTransactionIsolationCommits() throws SQLException {
-        // In the current client implementation, the transaction will
-        // commit when setTransactionIsolation is called, while the
-        // embedded driver will not commit. See
-        // http://issues.apache.org/jira/browse/DERBY-2064
+    public void testSetTransactionIsolationCommitRollback() throws SQLException {
         Connection conn = getConnection();
 
         conn.rollback();
@@ -166,26 +162,7 @@ public class SetTransactionIsolationTest
         ResultSet rs = s.executeQuery("select count(*) from t3");
         rs.next();
         int count = rs.getInt(1);
-        boolean passCommitCheck = false;
-        switch (count) {
-        case 1:
-            // Embedded and JCC don't commit
-            if (usingEmbedded())
-                passCommitCheck = true;
-            break;
-        case 2:
-            // Client commits
-            if (usingDerbyNetClient())
-                passCommitCheck = true;
-            break;
-        default:
-
-            fail("FAIL: count="
-                    + count
-                    + ", unexepected behaviour from testSetTransactionIsolationCommits");
-            break;
-        }
-        assertTrue(passCommitCheck);
+        assertEquals(1, count);
         rs.close();
         s.close();
 



Mime
View raw message