diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java index 8a1d4de7525..b5c42725104 100755 --- a/server/src/com/cloud/api/ApiServlet.java +++ b/server/src/com/cloud/api/ApiServlet.java @@ -122,6 +122,13 @@ public class ApiServlet extends HttpServlet { // utf8Fixup(req, params); + // logging the request start and end in management log for easy debugging + String reqStr = ""; + if (s_logger.isDebugEnabled()) { + reqStr = auditTrailSb.toString() + " " + req.getQueryString(); + s_logger.debug("===START=== " + reqStr); + } + try { HttpSession session = req.getSession(false); Object[] responseTypeParam = params.get("response"); @@ -335,6 +342,9 @@ public class ApiServlet extends HttpServlet { } } finally { s_accessLogger.info(auditTrailSb.toString()); + if (s_logger.isDebugEnabled()) { + s_logger.debug("===END=== " + reqStr); + } // cleanup user context to prevent from being peeked in other request context UserContext.unregisterContext(); } diff --git a/server/src/com/cloud/cluster/ClusterManagerImpl.java b/server/src/com/cloud/cluster/ClusterManagerImpl.java index 4dbb16c93c2..67af5ba4cb8 100755 --- a/server/src/com/cloud/cluster/ClusterManagerImpl.java +++ b/server/src/com/cloud/cluster/ClusterManagerImpl.java @@ -794,7 +794,8 @@ public class ClusterManagerImpl implements ClusterManager { invalidHeartbeatConnection(); } finally { - txn.close("ClusterHeartBeat"); + txn.transitToAutoManagedConnection(Transaction.CLOUD_DB); + txn.close("ClusterHeartBeat"); } } }; diff --git a/utils/src/com/cloud/utils/db/Transaction.java b/utils/src/com/cloud/utils/db/Transaction.java index bcf7ae1257a..3a669e11090 100755 --- a/utils/src/com/cloud/utils/db/Transaction.java +++ b/utils/src/com/cloud/utils/db/Transaction.java @@ -130,7 +130,7 @@ public class Transaction { // the existing DAO features // public void transitToUserManagedConnection(Connection conn) { - assert(_conn == null /*&& _stack.size() <= 1*/) : "Can't change to a user managed connection unless the stack is empty and the db connection is null: " + toString(); + assert(_conn == null /*&& _stack.size() <= 1*/) : "Can't change to a user managed connection unless the stack is empty and the db connection is null, you may have forgotten to invoke transitToAutoManagedConnection to close out the DB connection: " + toString(); _conn = conn; _dbId = CONNECTED_DB; } @@ -652,12 +652,6 @@ public class Transaction { s_logger.trace("Transaction is done"); cleanup(); } - - if(this._dbId == CONNECTED_DB) { - tls.set(_prev); - _prev = null; - s_mbean.removeTransaction(this); - } } /** @@ -753,14 +747,15 @@ public class Transaction { } try { - if (s_connLogger.isTraceEnabled()) { - s_connLogger.trace("Closing DB connection: dbconn" + System.identityHashCode(_conn)); - } + // we should only close db connection when it is not user managed if(this._dbId != CONNECTED_DB) { + if (s_connLogger.isTraceEnabled()) { + s_connLogger.trace("Closing DB connection: dbconn" + System.identityHashCode(_conn)); + } _conn.close(); + _conn = null; } - _conn = null; } catch (final SQLException e) { s_logger.warn("Unable to close connection", e); } diff --git a/utils/test/com/cloud/utils/db/DbTestDao.java b/utils/test/com/cloud/utils/db/DbTestDao.java new file mode 100644 index 00000000000..9530b3b2d44 --- /dev/null +++ b/utils/test/com/cloud/utils/db/DbTestDao.java @@ -0,0 +1,66 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.utils.db; + +import java.sql.PreparedStatement; +import java.util.Date; +import java.util.TimeZone; + +import com.cloud.utils.DateUtil; +import com.cloud.utils.exception.CloudRuntimeException; + +public class DbTestDao extends GenericDaoBase implements GenericDao { + protected DbTestDao() { + } + + @DB + public void create(int fldInt, long fldLong, String fldString) { + Transaction txn = Transaction.currentTxn(); + PreparedStatement pstmt = null; + try { + txn.start(); + pstmt = txn + .prepareAutoCloseStatement("insert into cloud.test(fld_int, fld_long, fld_string) values(?, ?, ?)"); + pstmt.setInt(1, fldInt); + pstmt.setLong(2, fldLong); + pstmt.setString(3, fldString); + + pstmt.executeUpdate(); + txn.commit(); + } catch (Exception e) { + throw new CloudRuntimeException("Problem with creating a record in test table", e); + } + } + + @DB + public void update(int fldInt, long fldLong, String fldString) { + Transaction txn = Transaction.currentTxn(); + PreparedStatement pstmt = null; + try { + txn.start(); + pstmt = txn.prepareAutoCloseStatement("update cloud.test set fld_int=?, fld_long=? where fld_string=?"); + pstmt.setInt(1, fldInt); + pstmt.setLong(2, fldLong); + pstmt.setString(3, fldString); + + pstmt.executeUpdate(); + txn.commit(); + } catch (Exception e) { + throw new CloudRuntimeException("Problem with creating a record in test table", e); + } + } +} diff --git a/utils/test/com/cloud/utils/db/DbTestVO.java b/utils/test/com/cloud/utils/db/DbTestVO.java new file mode 100644 index 00000000000..5285bfe50fe --- /dev/null +++ b/utils/test/com/cloud/utils/db/DbTestVO.java @@ -0,0 +1,56 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.utils.db; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +@Entity +@Table(name = "test") +public class DbTestVO { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + long id; + + @Column(name = "fld_int") + int fieldInt; + + @Column(name = "fld_long") + Long fieldLong; + + @Column(name = "fld_string") + String fieldString; + + public String getFieldString() { + return fieldString; + } + + public int getFieldInt() { + return fieldInt; + } + + public long getFieldLong() { + return fieldLong; + } + + public DbTestVO() { + } +} diff --git a/utils/test/com/cloud/utils/db/TransactionTest.java b/utils/test/com/cloud/utils/db/TransactionTest.java new file mode 100644 index 00000000000..2c79c587f3d --- /dev/null +++ b/utils/test/com/cloud/utils/db/TransactionTest.java @@ -0,0 +1,218 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.utils.db; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.cloud.utils.component.ComponentLocator; +import com.cloud.utils.db.QueryBuilderTest.TestDao; +import com.cloud.utils.db.QueryBuilderTest.TestVO; +import com.cloud.utils.exception.CloudRuntimeException; + +/** + * A test fixture to test APIs or bugs found for Transaction class. This test fixture will do one time setup before + * all its testcases to set up a test db table, and then tear down these test db artifacts after all testcases are run. + * + * @author Min Chen + * + */ +public class TransactionTest { + + @BeforeClass + public static void oneTimeSetup() { + Connection conn = null; + PreparedStatement pstmt = null; + try { + conn = Transaction.getStandaloneConnection(); + + pstmt = conn.prepareStatement("CREATE TABLE `cloud`.`test` (" + + "`id` bigint unsigned NOT NULL UNIQUE AUTO_INCREMENT," + "`fld_int` int unsigned," + + "`fld_long` bigint unsigned," + "`fld_string` varchar(255)," + "PRIMARY KEY (`id`)" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8;"); + + pstmt.execute(); + + } catch (SQLException e) { + throw new CloudRuntimeException("Problem with sql", e); + } finally { + if (pstmt != null) { + try { + pstmt.close(); + } catch (SQLException e) { + } + } + if (conn != null) { + try { + conn.close(); + } catch (SQLException e) { + } + } + } + } + + @Test + /** + * When a transaction is set to use user-managed db connection, for each following db statement, we should see + * that the same db connection is reused rather than acquiring a new one each time in typical transaction model. + */ + public void testUserManagedConnection() { + DbTestDao testDao = ComponentLocator.inject(DbTestDao.class); + Transaction txn = Transaction.open("SingleConnectionThread"); + Connection conn = null; + try { + conn = Transaction.getStandaloneConnectionWithException(); + txn.transitToUserManagedConnection(conn); + // try two SQLs to make sure that they are using the same connection + // acquired above. + testDao.create(1, 1, "Record 1"); + Connection checkConn = Transaction.currentTxn().getConnection(); + if (checkConn != conn) { + Assert.fail("A new db connection is acquired instead of using old one after create sql"); + } + testDao.update(2, 2, "Record 1"); + Connection checkConn2 = Transaction.currentTxn().getConnection(); + if (checkConn2 != conn) { + Assert.fail("A new db connection is acquired instead of using old one after update sql"); + } + } catch (SQLException e) { + Assert.fail(e.getMessage()); + } finally { + txn.transitToAutoManagedConnection(Transaction.CLOUD_DB); + txn.close(); + + if (conn != null) { + try { + conn.close(); + } catch (SQLException e) { + throw new CloudRuntimeException("Problem with close db connection", e); + } + } + } + } + + @Test + /** + * This test is simulating ClusterHeartBeat process, where the same transaction and db connection is reused. + */ + public void testTransactionReuse() { + DbTestDao testDao = ComponentLocator.inject(DbTestDao.class); + // acquire a db connection and keep it + Connection conn = null; + try { + conn = Transaction.getStandaloneConnectionWithException(); + } catch (SQLException ex) { + throw new CloudRuntimeException("Problem with getting db connection", ex); + } + + // start heartbeat loop, make sure that each loop still use the same + // connection + Transaction txn = null; + for (int i = 0; i < 3; i++) { + txn = Transaction.open("HeartbeatSimulator"); + try { + + txn.transitToUserManagedConnection(conn); + testDao.create(i, i, "Record " + i); + Connection checkConn = Transaction.currentTxn().getConnection(); + if (checkConn != conn) { + Assert.fail("A new db connection is acquired instead of using old one in loop " + i); + } + } catch (SQLException e) { + Assert.fail(e.getMessage()); + } finally { + txn.transitToAutoManagedConnection(Transaction.CLOUD_DB); + txn.close(); + } + } + // close the connection once we are done since we are managing db + // connection ourselves. + if (conn != null) { + try { + conn.close(); + } catch (SQLException e) { + throw new CloudRuntimeException("Problem with close db connection", e); + } + } + } + + @After + /** + * Delete all records after each test, but table is still kept + */ + public void tearDown() { + Connection conn = null; + PreparedStatement pstmt = null; + try { + conn = Transaction.getStandaloneConnection(); + + pstmt = conn.prepareStatement("truncate table `cloud`.`test`"); + pstmt.execute(); + + } catch (SQLException e) { + throw new CloudRuntimeException("Problem with sql", e); + } finally { + if (pstmt != null) { + try { + pstmt.close(); + } catch (SQLException e) { + } + } + if (conn != null) { + try { + conn.close(); + } catch (SQLException e) { + } + } + } + } + + @AfterClass + public static void oneTimeTearDown() { + Connection conn = null; + PreparedStatement pstmt = null; + try { + conn = Transaction.getStandaloneConnection(); + + pstmt = conn.prepareStatement("DROP TABLE IF EXISTS `cloud`.`test`"); + pstmt.execute(); + + } catch (SQLException e) { + throw new CloudRuntimeException("Problem with sql", e); + } finally { + if (pstmt != null) { + try { + pstmt.close(); + } catch (SQLException e) { + } + } + if (conn != null) { + try { + conn.close(); + } catch (SQLException e) { + } + } + } + } +}