You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ts...@apache.org on 2012/10/27 12:02:26 UTC
[17/49] git commit: CLOUDSTACK-409: ThreadLocal Transaction and its
db connection got reset for user managed db connnection,
causing ClusterHeartBeat thread frequently trying to get db connection. Add
unit test to test user managed transaction.
CLOUDSTACK-409: ThreadLocal Transaction and its db connection got reset for user managed db connnection, causing ClusterHeartBeat thread frequently trying to get db connection. Add unit test to test user managed transaction.
Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/7b7f4cd1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/7b7f4cd1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/7b7f4cd1
Branch: refs/heads/marvin-parallel
Commit: 7b7f4cd1fd72ece80a7efecf6d841b293773f726
Parents: e531763
Author: Min Chen <mi...@citrix.com>
Authored: Wed Oct 24 11:27:52 2012 -0700
Committer: Alex Huang <al...@citrix.com>
Committed: Thu Oct 25 13:06:50 2012 -0700
----------------------------------------------------------------------
server/src/com/cloud/api/ApiServlet.java | 10 +
.../src/com/cloud/cluster/ClusterManagerImpl.java | 3 +-
utils/src/com/cloud/utils/db/Transaction.java | 17 +-
utils/test/com/cloud/utils/db/DbTestDao.java | 66 +++++
utils/test/com/cloud/utils/db/DbTestVO.java | 56 ++++
utils/test/com/cloud/utils/db/TransactionTest.java | 218 +++++++++++++++
6 files changed, 358 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/server/src/com/cloud/api/ApiServlet.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java
index 8a1d4de..b5c4272 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();
}
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/server/src/com/cloud/cluster/ClusterManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/cluster/ClusterManagerImpl.java b/server/src/com/cloud/cluster/ClusterManagerImpl.java
index 4dbb16c..67af5ba 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");
}
}
};
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/utils/src/com/cloud/utils/db/Transaction.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/db/Transaction.java b/utils/src/com/cloud/utils/db/Transaction.java
index bcf7ae1..3a669e1 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);
}
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/utils/test/com/cloud/utils/db/DbTestDao.java
----------------------------------------------------------------------
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 0000000..9530b3b
--- /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<DbTestVO, Long> implements GenericDao<DbTestVO, Long> {
+ 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);
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/utils/test/com/cloud/utils/db/DbTestVO.java
----------------------------------------------------------------------
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 0000000..5285bfe
--- /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() {
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/7b7f4cd1/utils/test/com/cloud/utils/db/TransactionTest.java
----------------------------------------------------------------------
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 0000000..2c79c58
--- /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) {
+ }
+ }
+ }
+ }
+}