You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by kr...@apache.org on 2008/02/29 15:59:35 UTC

svn commit: r632334 - in /db/derby/code/trunk/java/client/org/apache/derby/client: am/Connection.java am/LogicalConnection.java am/LogicalStatementEntity.java am/PreparedStatement.java am/Statement.java net/NetConnection.java

Author: kristwaa
Date: Fri Feb 29 06:59:32 2008
New Revision: 632334

URL: http://svn.apache.org/viewvc?rev=632334&view=rev
Log:
DERBY-3441: Determine and implement a proper procedure for resetting a prepared statement for reuse in a statement pool.
Patch file: derby-3441-1c-statement_reset.diff

Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalStatementEntity.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.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=632334&r1=632333&r2=632334&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 Fri Feb 29 06:59:32 2008
@@ -273,6 +273,7 @@
                                    String user,
                                    ClientBaseDataSource ds,
                                    boolean recomputeFromDataSource) throws SqlException {
+        // Transaction isolation level is handled in completeReset.
         // clearWarningsX() will re-initialize the following properties
         clearWarningsX();
 
@@ -292,7 +293,6 @@
             // property: open_
             // this should already be true
 
-            isolation_ = TRANSACTION_UNKNOWN;
             currentSchemaName_ = null;
             autoCommit_ = true;
             inUnitOfWork_ = false;
@@ -822,7 +822,7 @@
                         e.getSQLException(), accumulatedExceptions);
         }
 
-        markClosed();
+        markClosed(false);
         try {
             agent_.close();
         } catch (SqlException e) {
@@ -835,7 +835,8 @@
 
     // Just like closeX except the socket is not pulled.
     // Physical resources are not closed.
-    synchronized public void closeForReuse() throws SqlException {
+    synchronized public void closeForReuse(boolean statementPooling)
+            throws SqlException {
         if (!open_) {
             return;
         }
@@ -847,7 +848,7 @@
             accumulatedExceptions = e;
         }
         if (open_) {
-            markClosedForReuse();
+            markClosedForReuse(statementPooling);
         }
         if (accumulatedExceptions != null) {
             throw accumulatedExceptions;
@@ -874,19 +875,21 @@
 
     protected abstract void markClosed_();
 
-    public void markClosed() // called by LogicalConnection.close()
+    public void markClosed(boolean statementPooling) // called by LogicalConnection.close()
     {
         open_ = false;
         inUnitOfWork_ = false;
-        markStatementsClosed();
+        if (!statementPooling) {
+            markStatementsClosed();
+        }
         CommitAndRollbackListeners_.clear();
         markClosed_();
     }
 
 
-    private void markClosedForReuse() {
+    private void markClosedForReuse(boolean statementPooling) {
         availableForReuse_ = true;
-        markClosed();
+        markClosed(statementPooling);
     }
 
     private void markStatementsClosed() {
@@ -2158,6 +2161,21 @@
             ClientBaseDataSource ds, 
             boolean recomputerFromDataSource) throws SqlException;
 
+    /**
+     * <br>NOTE:</br>The following comments are valid for the changes done as
+     * part of implementing statement caching only (see DERBY-3313 and linked
+     * issues).
+     * <p>
+     * We don't reset the isolation level to unknown unconditionally, as this
+     * forces us to go to the server all the time. Since the value should now
+     * be valid (DERBY-3192), we check if it has been changed from the default.
+     *
+     * @param recomputeFromDataSource is now used to differentiate between
+     *      cases where statement pooling is enabled or not. If {@code true}, it
+     *      means statement pooling is disabled and the statements are fully
+     *      reset, which includes a re-prepare. If {@code false}, statement
+     *      pooling is enabled, and a more lightweight reset procedure is used.
+     */
     protected void completeReset(boolean isDeferredReset, boolean recomputeFromDataSource) throws SqlException {
         open_ = true;
 
@@ -2167,11 +2185,25 @@
         // Notice that these physical statements may not belong to this logical connection.
         // Iterate through the physical statements and re-enable them for reuse.
 
-        java.util.Set keySet = openStatements_.keySet();
-        for (java.util.Iterator i = keySet.iterator(); i.hasNext();) {
-            Object o = i.next();
-            ((Statement) o).reset(recomputeFromDataSource);
-
+        if (recomputeFromDataSource) {
+            // 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(recomputeFromDataSource);
+            }
+        } 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);
+            }
         }
 
         if (!isDeferredReset && agent_.loggingEnabled()) {

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java?rev=632334&r1=632333&r2=632334&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java Fri Feb 29 06:59:32 2008
@@ -77,7 +77,8 @@
                     new ClientMessageId(
                         SQLState.PHYSICAL_CONNECTION_ALREADY_CLOSED)));
             } else {
-                physicalConnection_.closeForReuse();
+                physicalConnection_.closeForReuse(
+                        pooledConnection_.isStatementPoolingEnabled());
                 if (!physicalConnection_.isGlobalPending_()) {
                     pooledConnection_.recycleConnection();
                 }
@@ -105,7 +106,8 @@
                 ; // no call to recycleConnection()
             }
         } finally {
-            physicalConnection_.closeForReuse();  //poolfix
+            physicalConnection_.closeForReuse(
+                    pooledConnection_.isStatementPoolingEnabled());  //poolfix
             physicalConnection_ = null;
         }
     }

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalStatementEntity.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalStatementEntity.java?rev=632334&r1=632333&r2=632334&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalStatementEntity.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalStatementEntity.java Fri Feb 29 06:59:32 2008
@@ -172,10 +172,9 @@
 
             // Reset the statement for reuse.
             try {
-                // WARNING: This is just a placeholder and is incorrect!!!
-                //          A proper reset procedure must be implemented.
-                temporaryPsRef.reset(true);
+                temporaryPsRef.resetForReuse();
             } catch (SqlException sqle) {
+                // Get a wrapper and throw it.
                 throw sqle.getSQLException();
             }
 

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java?rev=632334&r1=632333&r2=632334&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/PreparedStatement.java Fri Feb 29 06:59:32 2008
@@ -111,6 +111,18 @@
         }
     }
 
+    /**
+     * Resets the prepared statement for reuse in a statement pool.
+     *
+     * @throws SqlException if the reset fails
+     * @see Statement#resetForReuse
+     */
+    void resetForReuse()
+            throws SqlException {
+        resetParameters();
+        super.resetForReuse();
+    }
+
     private void resetParameters() {
         if (parameterMetaData_ != null) {
             Arrays.fill(parameters_, null);

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java?rev=632334&r1=632333&r2=632334&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Statement.java Fri Feb 29 06:59:32 2008
@@ -247,21 +247,14 @@
         resultSet_ = null;
         updateCount_ = -1;
         returnValueFromProcedure_ = 0;
-        cursorName_ = null;
         openOnClient_ = true;
         openOnServer_ = false;
         indexOfCurrentResultSet_ = -1;
         resultSetList_ = null;
-        timeout_ = 0;
-        doWriteTimeout = false;
-        maxRows_ = 0;
-        maxFieldSize_ = 0;
         isCatalogQuery_ = false;
         isAutoCommittableStatement_ = true;
 
         batch_.clear();
-        fetchSize_ = 0;
-        fetchDirection_ = java.sql.ResultSet.FETCH_FORWARD;
         singletonRowData_ = null;
         numInvisibleRS_ = 0;
         preparedStatementForAutoGeneratedKeys_ = null;
@@ -270,8 +263,23 @@
         generatedKeysColumnIndexes_ = null;
         autoGeneratedKeys_ = java.sql.Statement.NO_GENERATED_KEYS;
 
-        // these members were not initialized
-        isPreparedStatement_ = false;
+        resetUserControllableAttributes();
+    }
+
+    /**
+     * Resets attributes that can be modified by the user through the
+     * {@link java.sql.Statement} interface to default values.
+     */
+    private void resetUserControllableAttributes() {
+        cursorName_ = null; // setCursorName
+        timeout_ = 0; // setQueryTimeout
+        doWriteTimeout = false; // setQueryTimeout
+        maxRows_ = 0; // setMaxRows
+        maxFieldSize_ = 0; // setMaxFieldSize
+        fetchSize_ = 0; // setFetchSize
+        fetchDirection_ = java.sql.ResultSet.FETCH_FORWARD; // setFetchDirection
+        isPoolable = isPreparedStatement_;
+        // Calls to setEscapeProcessing is currently a noop, nothing to reset.
     }
 
     // If a dataSource is passed into resetClientConnection(), then we will assume
@@ -288,6 +296,62 @@
         } else {
             initResetStatement();
             materialStatement_.reset_();
+        }
+    }
+
+    /**
+     * Resets the statement for reuse in a statement pool.
+     * <p>
+     * Intended to be used only by prepared or callable statements, as
+     * {@link java.sql.Statement} objects aren't pooled.
+     * <p>
+     * The following actions are taken:
+     * <ul> <li>Batches are cleared.</li>
+     *      <li>Warnings are cleared.</li>
+     *      <li>Open result set are closed on the client and the server.</li>
+     *      <li>Cached cursor names are cleared.</li>
+     *      <li>Statement for fetching auto-generated keys is closed.</li>
+     *      <li>Special registers are reset.</li>
+     *      <li>User controllable attributes are reset to defaults, for instance
+     *          query timeout and max rows to fetch.</li>
+     * </ul>
+     *
+     * @throws SqlException if resetting the statement fails
+     */
+    void resetForReuse()
+            throws SqlException {
+        this.batch_.clear();
+        clearWarningsX();
+        // Close open resultsets.
+        // Regardless of whether or not this statement is in the prepared state,
+        // we need to close any open cursors for this statement on the server.
+        int numberOfResultSetsToClose = 0;
+        if (resultSetList_ != null) {
+            numberOfResultSetsToClose = resultSetList_.length;
+        }
+        try {
+            if (willTickleServer(
+                    numberOfResultSetsToClose, connection_.autoCommit_)) {
+                flowClose();
+            } else {
+                flowCloseOutsideUOW();
+            }
+        } finally {
+            // If an exception is thrown above, the statement should not be put
+            // back into the statement pool, as the state may not be consistent.
+            markResultSetsClosed();
+            // In case a cursorName was set on the Statement but the Statement
+            // was never used to execute a query, the cursorName will not be
+            // remove when the resultSets are mark closed, so we need to remove
+            // the cursorName from the cache.
+            removeClientCursorNameFromCache();
+            markPreparedStatementForAutoGeneratedKeysClosed();
+
+            if (setSpecialRegisterSection_ != null) {
+                setSpecialRegisterSection_.free();
+                setSpecialRegisterSection_ = null;
+            }
+            resetUserControllableAttributes();
         }
     }
 

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=632334&r1=632333&r2=632334&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 Fri Feb 29 06:59:32 2008
@@ -51,8 +51,8 @@
     //The PreparedStatement then uses this to pass the close and the error
     //occurred conditions back to the PooledConnection which can then throw the 
     //appropriate events.
-    protected ClientPooledConnection pooledConnection_ = null;
-
+    private final ClientPooledConnection pooledConnection_;
+    private final boolean closeStatementsOnClose;
 
     // For XA Transaction
     protected int pendingEndXACallinfoOffset_ = -1;
@@ -185,6 +185,8 @@
                          String databaseName,
                          java.util.Properties properties) throws SqlException {
         super(netLogWriter, 0, "", -1, databaseName, properties);
+        this.pooledConnection_ = null;
+        this.closeStatementsOnClose = true;
     }
 
     public NetConnection(NetLogWriter netLogWriter,
@@ -192,6 +194,8 @@
                          String user,
                          String password) throws SqlException {
         super(netLogWriter, user, password, dataSource);
+        this.pooledConnection_ = null;
+        this.closeStatementsOnClose = true;
         setDeferredResetPassword(password);
     }
 
@@ -203,6 +207,8 @@
                          String databaseName,
                          java.util.Properties properties) throws SqlException {
         super(netLogWriter, driverManagerLoginTimeout, serverName, portNumber, databaseName, properties);
+        this.pooledConnection_ = null;
+        this.closeStatementsOnClose = true;
         netAgent_ = (NetAgent) super.agent_;
         if (netAgent_.exceptionOpeningSocket_ != null) {
             throw netAgent_.exceptionOpeningSocket_;
@@ -223,6 +229,8 @@
                          int rmId,
                          boolean isXAConn) throws SqlException {
         super(netLogWriter, user, password, isXAConn, dataSource);
+        this.pooledConnection_ = null;
+        this.closeStatementsOnClose = true;
         netAgent_ = (NetAgent) super.agent_;
         initialize(user, password, dataSource, rmId, isXAConn);
     }
@@ -233,6 +241,8 @@
                          org.apache.derby.jdbc.ClientBaseDataSource dataSource,
                          boolean isXAConn) throws SqlException {
         super(netLogWriter, isXAConn, dataSource);
+        this.pooledConnection_ = null;
+        this.closeStatementsOnClose = true;
         netAgent_ = (NetAgent) super.agent_;
         if (netAgent_.exceptionOpeningSocket_ != null) {
             throw netAgent_.exceptionOpeningSocket_;
@@ -275,6 +285,7 @@
         netAgent_ = (NetAgent) super.agent_;
         initialize(user, password, dataSource, rmId, isXAConn);
         this.pooledConnection_=cpc;
+        this.closeStatementsOnClose = !cpc.isStatementPoolingEnabled();
     }
 
     private void initialize(String user,
@@ -409,7 +420,12 @@
     }
 
     protected void completeReset(boolean isDeferredReset, boolean recomputeFromDataSource) throws SqlException {
-        super.completeReset(isDeferredReset, recomputeFromDataSource);
+        // NB! Override the recomputFromDataSource flag.
+        //     This was done as a temporary, minimal intrusive fix to support
+        //     JDBC statement pooling.
+        //     See DERBY-3341 for details.
+        super.completeReset(isDeferredReset,
+                recomputeFromDataSource && closeStatementsOnClose);
     }
 
     public void flowConnect(String password,
@@ -1550,7 +1566,7 @@
     }
 
     protected boolean doCloseStatementsOnClose_() {
-        return true;
+        return closeStatementsOnClose;
     }
 
     protected boolean allowCloseInUOW_() {
@@ -1796,7 +1812,7 @@
      */
     synchronized public void closeForReuse() throws SqlException {
         // call super.close*() to do the close*
-        super.closeForReuse();
+        super.closeForReuse(closeStatementsOnClose);
         if (!isXAConnection_)
             return;
         if (isOpen()) {