Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1935#discussion_r102535697
--- Diff: server/src/com/cloud/user/DomainManagerImpl.java ---
@@ -273,82 +289,133 @@ 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 = getGlobalLock("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<Long> networkIds =
_networkDomainDao.listNetworkIdsByDomain(domain.getId());
- List<AccountVO> accountsForCleanup =
_accountDao.findCleanupsForRemovedAccounts(domain.getId());
- List<DedicatedResourceVO> 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);
+
+ try {
+ long ownerId = domain.getAccountId();
+ if (BooleanUtils.toBoolean(cleanup)) {
+ if (!cleanupDomain(domain.getId(), ownerId)) {
--- End diff --
Ah, the naming of methods, I also find very hard sometimes; I think most
programmers have this problem. I think the name you proposed is ok. I am not
creative today.
I also liked the way the code became cleaner and easier to read. It also
facilitates future changes.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
