Skip to content

Commit 7b7f4cd

Browse files
minchen07Alex Huang
minchen07
authored and
Alex Huang
committed
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.
1 parent e531763 commit 7b7f4cd

6 files changed

Lines changed: 358 additions & 12 deletions

File tree

‎server/src/com/cloud/api/ApiServlet.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ private void processRequest(HttpServletRequest req, HttpServletResponse resp) {
122122
//
123123
utf8Fixup(req, params);
124124

125+
// logging the request start and end in management log for easy debugging
126+
String reqStr = "";
127+
if (s_logger.isDebugEnabled()) {
128+
reqStr = auditTrailSb.toString() + " " + req.getQueryString();
129+
s_logger.debug("===START=== " + reqStr);
130+
}
131+
125132
try {
126133
HttpSession session = req.getSession(false);
127134
Object[] responseTypeParam = params.get("response");
@@ -335,6 +342,9 @@ private void processRequest(HttpServletRequest req, HttpServletResponse resp) {
335342
}
336343
} finally {
337344
s_accessLogger.info(auditTrailSb.toString());
345+
if (s_logger.isDebugEnabled()) {
346+
s_logger.debug("===END=== " + reqStr);
347+
}
338348
// cleanup user context to prevent from being peeked in other request context
339349
UserContext.unregisterContext();
340350
}

‎server/src/com/cloud/cluster/ClusterManagerImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,8 @@ public void run() {
794794

795795
invalidHeartbeatConnection();
796796
} finally {
797-
txn.close("ClusterHeartBeat");
797+
txn.transitToAutoManagedConnection(Transaction.CLOUD_DB);
798+
txn.close("ClusterHeartBeat");
798799
}
799800
}
800801
};

‎utils/src/com/cloud/utils/db/Transaction.java‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public static Transaction open(final short databaseId) {
130130
// the existing DAO features
131131
//
132132
public void transitToUserManagedConnection(Connection conn) {
133-
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();
133+
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();
134134
_conn = conn;
135135
_dbId = CONNECTED_DB;
136136
}
@@ -652,12 +652,6 @@ public void close() {
652652
s_logger.trace("Transaction is done");
653653
cleanup();
654654
}
655-
656-
if(this._dbId == CONNECTED_DB) {
657-
tls.set(_prev);
658-
_prev = null;
659-
s_mbean.removeTransaction(this);
660-
}
661655
}
662656

663657
/**
@@ -753,14 +747,15 @@ protected void closeConnection() {
753747
}
754748

755749
try {
756-
if (s_connLogger.isTraceEnabled()) {
757-
s_connLogger.trace("Closing DB connection: dbconn" + System.identityHashCode(_conn));
758-
}
750+
// we should only close db connection when it is not user managed
759751
if(this._dbId != CONNECTED_DB) {
752+
if (s_connLogger.isTraceEnabled()) {
753+
s_connLogger.trace("Closing DB connection: dbconn" + System.identityHashCode(_conn));
754+
}
760755
_conn.close();
756+
_conn = null;
761757
}
762758

763-
_conn = null;
764759
} catch (final SQLException e) {
765760
s_logger.warn("Unable to close connection", e);
766761
}

‎utils/test/com/cloud/utils/db/DbTestDao.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.utils.db;
18+
19+
import java.sql.PreparedStatement;
20+
import java.util.Date;
21+
import java.util.TimeZone;
22+
23+
import com.cloud.utils.DateUtil;
24+
import com.cloud.utils.exception.CloudRuntimeException;
25+
26+
public class DbTestDao extends GenericDaoBase<DbTestVO, Long> implements GenericDao<DbTestVO, Long> {
27+
protected DbTestDao() {
28+
}
29+
30+
@DB
31+
public void create(int fldInt, long fldLong, String fldString) {
32+
Transaction txn = Transaction.currentTxn();
33+
PreparedStatement pstmt = null;
34+
try {
35+
txn.start();
36+
pstmt = txn
37+
.prepareAutoCloseStatement("insert into cloud.test(fld_int, fld_long, fld_string) values(?, ?, ?)");
38+
pstmt.setInt(1, fldInt);
39+
pstmt.setLong(2, fldLong);
40+
pstmt.setString(3, fldString);
41+
42+
pstmt.executeUpdate();
43+
txn.commit();
44+
} catch (Exception e) {
45+
throw new CloudRuntimeException("Problem with creating a record in test table", e);
46+
}
47+
}
48+
49+
@DB
50+
public void update(int fldInt, long fldLong, String fldString) {
51+
Transaction txn = Transaction.currentTxn();
52+
PreparedStatement pstmt = null;
53+
try {
54+
txn.start();
55+
pstmt = txn.prepareAutoCloseStatement("update cloud.test set fld_int=?, fld_long=? where fld_string=?");
56+
pstmt.setInt(1, fldInt);
57+
pstmt.setLong(2, fldLong);
58+
pstmt.setString(3, fldString);
59+
60+
pstmt.executeUpdate();
61+
txn.commit();
62+
} catch (Exception e) {
63+
throw new CloudRuntimeException("Problem with creating a record in test table", e);
64+
}
65+
}
66+
}

‎utils/test/com/cloud/utils/db/DbTestVO.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.utils.db;
18+
19+
import javax.persistence.Column;
20+
import javax.persistence.Entity;
21+
import javax.persistence.GeneratedValue;
22+
import javax.persistence.GenerationType;
23+
import javax.persistence.Id;
24+
import javax.persistence.Table;
25+
26+
@Entity
27+
@Table(name = "test")
28+
public class DbTestVO {
29+
@Id
30+
@GeneratedValue(strategy = GenerationType.IDENTITY)
31+
long id;
32+
33+
@Column(name = "fld_int")
34+
int fieldInt;
35+
36+
@Column(name = "fld_long")
37+
Long fieldLong;
38+
39+
@Column(name = "fld_string")
40+
String fieldString;
41+
42+
public String getFieldString() {
43+
return fieldString;
44+
}
45+
46+
public int getFieldInt() {
47+
return fieldInt;
48+
}
49+
50+
public long getFieldLong() {
51+
return fieldLong;
52+
}
53+
54+
public DbTestVO() {
55+
}
56+
}

0 commit comments

Comments
 (0)