Skip to content

Commit 1edb3e8

Browse files
subhash yedugundlayadvr
authored andcommitted
CLOUDSTACK-9595: Avoiding the deadlocks in the code (#1762)
MySQLTransactionRollbackException is seen frequently in logs Root Cause Attempts to lock rows in the core data access layer of database fails if there is a possibility of deadlock. However Operations are not getting retried in case of deadlock. So introducing retries here Solution Operations would be retried after some wait time in case of dead lock exception.
1 parent 3c6df7c commit 1edb3e8

File tree

2 files changed

+66
-63
lines changed

2 files changed

+66
-63
lines changed

engine/schema/src/com/cloud/host/dao/HostDaoImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,6 @@ private boolean canOwnCluster(long clusterId) {
553553
public List<HostVO> findAndUpdateDirectAgentToLoad(long lastPingSecondsAfter, Long limit, long managementServerId) {
554554
TransactionLegacy txn = TransactionLegacy.currentTxn();
555555

556-
txn.start();
557556
if (s_logger.isDebugEnabled()) {
558557
s_logger.debug("Resetting hosts suitable for reconnect");
559558
}
@@ -569,6 +568,7 @@ public List<HostVO> findAndUpdateDirectAgentToLoad(long lastPingSecondsAfter, Lo
569568
s_logger.debug("Acquiring hosts for clusters already owned by this management server");
570569
}
571570
List<Long> clusters = findClustersOwnedByManagementServer(managementServerId);
571+
txn.start();
572572
if (clusters.size() > 0) {
573573
// handle clusters already owned by @managementServerId
574574
SearchCriteria<HostVO> sc = UnmanagedDirectConnectSearch.create();

server/src/com/cloud/network/IpAddressManagerImpl.java

Lines changed: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
291291

292292
SearchBuilder<IPAddressVO> AssignIpAddressSearch;
293293
SearchBuilder<IPAddressVO> AssignIpAddressFromPodVlanSearch;
294+
private final Object _allocatedLock = new Object();
295+
private final Object _allocatingLock = new Object();
294296

295297
static Boolean rulesContinueOnErrFlag = true;
296298

@@ -759,7 +761,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
759761

760762
Filter filter = new Filter(IPAddressVO.class, "vlanId", true, 0l, 1l);
761763

762-
List<IPAddressVO> addrs = _ipAddressDao.lockRows(sc, filter, true);
764+
List<IPAddressVO> addrs = _ipAddressDao.search(sc, filter, false);
763765

764766
// If all the dedicated IPs of the owner are in use fetch an IP from the system pool
765767
if (addrs.size() == 0 && fetchFromDedicatedRange) {
@@ -769,7 +771,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
769771
fetchFromDedicatedRange = false;
770772
sc.setParameters("vlanId", nonDedicatedVlanDbIds.toArray());
771773
errorMessage.append(", vlanId id=" + Arrays.toString(nonDedicatedVlanDbIds.toArray()));
772-
addrs = _ipAddressDao.lockRows(sc, filter, true);
774+
addrs = _ipAddressDao.search(sc, filter, false);
773775
}
774776
}
775777

@@ -804,24 +806,21 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
804806
addr.setAllocatedInDomainId(owner.getDomainId());
805807
addr.setAllocatedToAccountId(owner.getId());
806808
addr.setSystem(isSystem);
809+
807810
if (displayIp != null) {
808811
addr.setDisplay(displayIp);
809812
}
810813

811-
if (assign) {
812-
markPublicIpAsAllocated(addr);
813-
} else {
814-
addr.setState(IpAddress.State.Allocating);
815-
}
816-
addr.setState(assign ? IpAddress.State.Allocated : IpAddress.State.Allocating);
817-
818814
if (vlanUse != VlanType.DirectAttached) {
819815
addr.setAssociatedWithNetworkId(guestNetworkId);
820816
addr.setVpcId(vpcId);
821817
}
822818

823-
_ipAddressDao.update(addr.getId(), addr);
824-
819+
if (assign) {
820+
markPublicIpAsAllocated(addr);
821+
} else {
822+
markPublicIpAsAllocating(addr);
823+
}
825824
return addr;
826825
}
827826
});
@@ -836,35 +835,51 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
836835
@DB
837836
@Override
838837
public void markPublicIpAsAllocated(final IPAddressVO addr) {
839-
840-
Transaction.execute(new TransactionCallbackNoReturn() {
841-
@Override
842-
public void doInTransactionWithoutResult(TransactionStatus status) {
843-
Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
844-
synchronized (this) {
838+
synchronized (_allocatedLock) {
839+
Transaction.execute(new TransactionCallbackNoReturn() {
840+
@Override
841+
public void doInTransactionWithoutResult(TransactionStatus status) {
842+
Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
845843
if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
846844
IPAddressVO userIp = _ipAddressDao.findById(addr.getId());
847845
if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free) {
848846
addr.setState(IpAddress.State.Allocated);
849-
_ipAddressDao.update(addr.getId(), addr);
850-
// Save usage event
851-
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
852-
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
853-
String guestType = vlan.getVlanType().toString();
854-
if (!isIpDedicated(addr)) {
855-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
856-
addr.getAddress().toString(),
857-
addr.isSourceNat(), guestType, addr.getSystem(), addr.getClass().getName(), addr.getUuid());
858-
}
859-
if (updateIpResourceCount(addr)) {
860-
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
847+
if (_ipAddressDao.update(addr.getId(), addr)) {
848+
// Save usage event
849+
if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) {
850+
VlanVO vlan = _vlanDao.findById(addr.getVlanId());
851+
String guestType = vlan.getVlanType().toString();
852+
if (!isIpDedicated(addr)) {
853+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(),
854+
addr.getAddress().toString(),
855+
addr.isSourceNat(), guestType, addr.getSystem(), addr.getClass().getName(), addr.getUuid());
856+
}
857+
if (updateIpResourceCount(addr)) {
858+
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
859+
}
861860
}
862861
}
863862
}
864863
}
865864
}
866-
}
867-
});
865+
});
866+
}
867+
}
868+
869+
@DB
870+
private void markPublicIpAsAllocating(final IPAddressVO addr) {
871+
synchronized (_allocatingLock) {
872+
Transaction.execute(new TransactionCallbackNoReturn() {
873+
@Override
874+
public void doInTransactionWithoutResult(TransactionStatus status) {
875+
876+
if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
877+
addr.setState(IpAddress.State.Allocating);
878+
_ipAddressDao.update(addr.getId(), addr);
879+
}
880+
}
881+
});
882+
}
868883
}
869884

870885
private boolean isIpDedicated(IPAddressVO addr) {
@@ -901,40 +916,28 @@ public PublicIp assignDedicateIpAddress(Account owner, final Long guestNtwkId, f
901916

902917
PublicIp ip = null;
903918
try {
904-
ip = Transaction.execute(new TransactionCallbackWithException<PublicIp, InsufficientAddressCapacityException>() {
905-
@Override
906-
public PublicIp doInTransaction(TransactionStatus status) throws InsufficientAddressCapacityException {
907-
Account owner = _accountDao.acquireInLockTable(ownerId);
908-
909-
if (owner == null) {
910-
// this ownerId comes from owner or type Account. See the class "AccountVO" and the annotations in that class
911-
// to get the table name and field name that is queried to fill this ownerid.
912-
ConcurrentOperationException ex = new ConcurrentOperationException("Unable to lock account");
913-
throw ex;
914-
}
915-
if (s_logger.isDebugEnabled()) {
916-
s_logger.debug("lock account " + ownerId + " is acquired");
917-
}
918-
boolean displayIp = true;
919-
if (guestNtwkId != null) {
920-
Network ntwk = _networksDao.findById(guestNtwkId);
921-
displayIp = ntwk.getDisplayNetwork();
922-
} else if (vpcId != null) {
923-
VpcVO vpc = _vpcDao.findById(vpcId);
924-
displayIp = vpc.isDisplay();
925-
}
919+
Account ownerAccount = _accountDao.acquireInLockTable(ownerId);
926920

927-
PublicIp ip = fetchNewPublicIp(dcId, null, null, owner, VlanType.VirtualNetwork, guestNtwkId, isSourceNat, false, null, false, vpcId, displayIp);
928-
IPAddressVO publicIp = ip.ip();
929-
930-
markPublicIpAsAllocated(publicIp);
931-
_ipAddressDao.update(publicIp.getId(), publicIp);
921+
if (ownerAccount == null) {
922+
// this ownerId comes from owner or type Account. See the class "AccountVO" and the annotations in that class
923+
// to get the table name and field name that is queried to fill this ownerid.
924+
ConcurrentOperationException ex = new ConcurrentOperationException("Unable to lock account");
925+
throw ex;
926+
}
927+
if (s_logger.isDebugEnabled()) {
928+
s_logger.debug("lock account " + ownerId + " is acquired");
929+
}
930+
boolean displayIp = true;
931+
if (guestNtwkId != null) {
932+
Network ntwk = _networksDao.findById(guestNtwkId);
933+
displayIp = ntwk.getDisplayNetwork();
934+
} else if (vpcId != null) {
935+
VpcVO vpc = _vpcDao.findById(vpcId);
936+
displayIp = vpc.isDisplay();
937+
}
932938

933-
return ip;
934-
}
935-
});
939+
return fetchNewPublicIp(dcId, null, null, owner, VlanType.VirtualNetwork, guestNtwkId, isSourceNat, false, null, false, vpcId, displayIp);
936940

937-
return ip;
938941
} finally {
939942
if (owner != null) {
940943
if (s_logger.isDebugEnabled()) {

0 commit comments

Comments
 (0)