Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 5815E200C22 for ; Tue, 21 Feb 2017 18:19:58 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 56D0A160B68; Tue, 21 Feb 2017 17:19:58 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 79FD5160B4F for ; Tue, 21 Feb 2017 18:19:57 +0100 (CET) Received: (qmail 61039 invoked by uid 500); 21 Feb 2017 17:19:56 -0000 Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list dev@cloudstack.apache.org Received: (qmail 61022 invoked by uid 99); 21 Feb 2017 17:19:56 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 21 Feb 2017 17:19:56 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 1D773DFDD1; Tue, 21 Feb 2017 17:19:56 +0000 (UTC) From: nvazquez To: dev@cloudstack.apache.org Reply-To: dev@cloudstack.apache.org References: In-Reply-To: Subject: [GitHub] cloudstack pull request #1935: CLOUDSTACK-9764: Delete domain failure due to... Content-Type: text/plain Message-Id: <20170221171956.1D773DFDD1@git1-us-west.apache.org> Date: Tue, 21 Feb 2017 17:19:56 +0000 (UTC) archived-at: Tue, 21 Feb 2017 17:19:58 -0000 Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1935#discussion_r102265150 --- Diff: server/src/com/cloud/user/DomainManagerImpl.java --- @@ -273,79 +274,97 @@ public boolean deleteDomain(long domainId, Boolean cleanup) { @Override public boolean deleteDomain(DomainVO domain, Boolean cleanup) { - // mark domain as inactive - s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it"); - domain.setState(Domain.State.Inactive); - _domainDao.update(domain.getId(), domain); - boolean rollBackState = false; - boolean hasDedicatedResources = false; + GlobalLock lock = GlobalLock.getInternLock("AccountCleanup"); + if (lock == null) { + s_logger.debug("Couldn't get the global lock"); + return false; + } + + if (!lock.lock(30)) { + s_logger.debug("Couldn't lock the db"); + return false; + } try { - long ownerId = domain.getAccountId(); - if ((cleanup != null) && cleanup.booleanValue()) { - if (!cleanupDomain(domain.getId(), ownerId)) { - rollBackState = true; - CloudRuntimeException e = - new CloudRuntimeException("Failed to clean up domain resources and sub domains, delete failed on domain " + domain.getName() + " (id: " + - domain.getId() + ")."); - e.addProxyObject(domain.getUuid(), "domainId"); - throw e; - } - } else { - //don't delete the domain if there are accounts set for cleanup, or non-removed networks exist, or domain has dedicated resources - List networkIds = _networkDomainDao.listNetworkIdsByDomain(domain.getId()); - List accountsForCleanup = _accountDao.findCleanupsForRemovedAccounts(domain.getId()); - List dedicatedResources = _dedicatedDao.listByDomainId(domain.getId()); - if (dedicatedResources != null && !dedicatedResources.isEmpty()) { - s_logger.error("There are dedicated resources for the domain " + domain.getId()); - hasDedicatedResources = true; - } - if (accountsForCleanup.isEmpty() && networkIds.isEmpty() && !hasDedicatedResources) { - _messageBus.publish(_name, MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain); - if (!_domainDao.remove(domain.getId())) { + // mark domain as inactive + s_logger.debug("Marking domain id=" + domain.getId() + " as " + Domain.State.Inactive + " before actually deleting it"); + domain.setState(Domain.State.Inactive); + _domainDao.update(domain.getId(), domain); + boolean rollBackState = false; + boolean hasDedicatedResources = false; + + try { + long ownerId = domain.getAccountId(); + if ((cleanup != null) && cleanup.booleanValue()) { + if (!cleanupDomain(domain.getId(), ownerId)) { rollBackState = true; CloudRuntimeException e = - new CloudRuntimeException("Delete failed on domain " + domain.getName() + " (id: " + domain.getId() + - "); Please make sure all users and sub domains have been removed from the domain before deleting"); + new CloudRuntimeException("Failed to clean up domain resources and sub domains, delete failed on domain " + domain.getName() + " (id: " + + domain.getId() + ")."); e.addProxyObject(domain.getUuid(), "domainId"); throw e; } - _messageBus.publish(_name, MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain); } else { - rollBackState = true; - String msg = null; - if (!accountsForCleanup.isEmpty()) { - msg = accountsForCleanup.size() + " accounts to cleanup"; - } else if (!networkIds.isEmpty()) { - msg = networkIds.size() + " non-removed networks"; - } else if (hasDedicatedResources) { - msg = "dedicated resources."; + //don't delete the domain if there are accounts set for cleanup, or non-removed networks exist, or domain has dedicated resources + List networkIds = _networkDomainDao.listNetworkIdsByDomain(domain.getId()); + List accountsForCleanup = _accountDao.findCleanupsForRemovedAccounts(domain.getId()); + List dedicatedResources = _dedicatedDao.listByDomainId(domain.getId()); + if (dedicatedResources != null && !dedicatedResources.isEmpty()) { + s_logger.error("There are dedicated resources for the domain " + domain.getId()); + hasDedicatedResources = true; + } + if (accountsForCleanup.isEmpty() && networkIds.isEmpty() && !hasDedicatedResources) { + _messageBus.publish(_name, MESSAGE_PRE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain); + if (!_domainDao.remove(domain.getId())) { + rollBackState = true; + CloudRuntimeException e = + new CloudRuntimeException("Delete failed on domain " + domain.getName() + " (id: " + domain.getId() + + "); Please make sure all users and sub domains have been removed from the domain before deleting"); + e.addProxyObject(domain.getUuid(), "domainId"); + throw e; + } + _messageBus.publish(_name, MESSAGE_REMOVE_DOMAIN_EVENT, PublishScope.LOCAL, domain); + } else { + rollBackState = true; + String msg = null; + if (!accountsForCleanup.isEmpty()) { + msg = accountsForCleanup.size() + " accounts to cleanup"; + } else if (!networkIds.isEmpty()) { + msg = networkIds.size() + " non-removed networks"; + } else if (hasDedicatedResources) { + msg = "dedicated resources."; + } + + CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because it has " + msg); + e.addProxyObject(domain.getUuid(), "domainId"); + throw e; } - - CloudRuntimeException e = new CloudRuntimeException("Can't delete the domain yet because it has " + msg); - e.addProxyObject(domain.getUuid(), "domainId"); - throw e; } - } - cleanupDomainOfferings(domain.getId()); - CallContext.current().putContextParameter(Domain.class, domain.getUuid()); - return true; - } catch (Exception ex) { - s_logger.error("Exception deleting domain with id " + domain.getId(), ex); - if (ex instanceof CloudRuntimeException) - throw (CloudRuntimeException)ex; - else - return false; - } finally { - //when success is false - if (rollBackState) { - s_logger.debug("Changing domain id=" + domain.getId() + " state back to " + Domain.State.Active + - " because it can't be removed due to resources referencing to it"); - domain.setState(Domain.State.Active); - _domainDao.update(domain.getId(), domain); + cleanupDomainOfferings(domain.getId()); + CallContext.current().putContextParameter(Domain.class, domain.getUuid()); + return true; + } catch (Exception ex) { + s_logger.error("Exception deleting domain with id " + domain.getId(), ex); + if (ex instanceof CloudRuntimeException) + throw (CloudRuntimeException)ex; + else + return false; + } finally { + lock.unlock(); + //when success is false + if (rollBackState) { + s_logger.debug("Changing domain id=" + domain.getId() + " state back to " + Domain.State.Active + + " because it can't be removed due to resources referencing to it"); + domain.setState(Domain.State.Active); + _domainDao.update(domain.getId(), domain); + } } } + catch (Exception e) { --- End diff -- Actually it wasn't, I removed catch block, thanks @koushik-das --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastructure@apache.org or file a JIRA ticket with INFRA. ---