You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by nvazquez <gi...@git.apache.org> on 2016/06/16 17:40:24 UTC
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
GitHub user nvazquez opened a pull request:
https://github.com/apache/cloudstack/pull/1594
CLOUDSTACK-9407: vm_network_map table doesnt get cleaned up properly
JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9407
### Introduction
It was found out that in production environments `vm_network_map` table entries were slowly growing. It was investigated how this entries were cleaned up.
### Behaviour
On vm creation, vm mappings are inserted on `vm_network_map`.
On vm stop, mappings are deleted from `vm_network_map` for vm, as a result of the release of its nics.
### Problem
If created vm is stopped from hypervisor side (at least on vSphere in which we tested it), when CloudStack realizes vm is stopped it doesn't clean up `vm_network_table,` and, as cleanup is made during vm stop, when vm is eventually destroyed and expunged it won't clean up their entries in that table.
### Proposed solution
We propose to move `vm_network_map` table cleanup to expunge command instead of stop command.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nvazquez/cloudstack vmnetworkmapissue
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1594.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1594
----
commit d3f3fb0590f14c540493d5f27c18cea13fa092af
Author: nvazquez <ni...@gmail.com>
Date: 2016-06-06T14:47:45Z
CLOUDSTACK-9407: Release network resources on expunge command
----
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67907159
--- Diff: server/src/com/cloud/vm/UserVmManagerImpl.java ---
@@ -2086,6 +2089,18 @@ public boolean expunge(UserVmVO vm, long callerUserId, Account caller) {
}
}
+ /**
+ * Release network resources, it was done on vm stop previously.
+ * @param id vm id
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ */
+ private void releaseNetworkResourcesOnExpunge(long id) throws ConcurrentOperationException, ResourceUnavailableException {
+ final VMInstanceVO vmInstance = _vmDao.findById(id);
--- End diff --
Done, thanks for pointing it!
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67571914
--- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
@@ -1577,8 +1577,9 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
}
try {
- _networkMgr.release(profile, cleanUpEvenIfUnableToStop);
- s_logger.debug("Successfully released network resources for the vm " + vm);
+ s_logger.debug("Not releasing network resources until expunge command is sent");
--- End diff --
Please include context information, such as descriptions and IDs of the network resources, in the log message.
---
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.
---
[GitHub] cloudstack issue #1594: CLOUDSTACK-9407: vm_network_map table doesnt get cle...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1594
@nvazquez I have fun this on two different CI environments with the same error coming up in both. Can you confirm this is not related to this change? Thanks...
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67907224
--- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
@@ -1577,8 +1577,9 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
}
try {
- _networkMgr.release(profile, cleanUpEvenIfUnableToStop);
- s_logger.debug("Successfully released network resources for the vm " + vm);
+ s_logger.debug("Not releasing network resources until expunge command is sent");
+ //_networkMgr.release(profile, cleanUpEvenIfUnableToStop);
+ //s_logger.debug("Successfully released network resources for the vm " + vm);
--- End diff --
Done, thanks!
---
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.
---
[GitHub] cloudstack issue #1594: CLOUDSTACK-9407: vm_network_map table doesnt get cle...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1594
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 1
Errors: 0
Duration: 3h 52m 54s
```
**Summary of the problem(s):**
```
FAIL: Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test Network GC Nics
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs2/cloudstack/test/integration/smoke/test_vpc_redundant.py", line 605, in test_04_rvpc_network_garbage_collector_nics
self.check_routers_state(status_to_check="BACKUP", expected_count=2)
File "/data/git/cs2/cloudstack/test/integration/smoke/test_vpc_redundant.py", line 353, in check_routers_state
self.fail("Expected '%s' routers at state '%s', but found '%s'!" % (expected_count, status_to_check, cnts[vals.index(status_to_check)]))
AssertionError: Expected '2' routers at state 'BACKUP', but found '1'!
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_2PRTDA/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__Jul_18_2016_07_15_50_GQQ4IO:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/DeployDataCenter__Jul_18_2016_07_15_50_GQQ4IO/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/DeployDataCenter__Jul_18_2016_07_15_50_GQQ4IO/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/DeployDataCenter__Jul_18_2016_07_15_50_GQQ4IO/runinfo.txt)
**`/tmp/MarvinLogs/test_network_2PRTDA:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_network_2PRTDA/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_network_2PRTDA/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_network_2PRTDA/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_NTI7U6:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_vpc_routers_NTI7U6/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_vpc_routers_NTI7U6/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1594/tmp/MarvinLogs/test_vpc_routers_NTI7U6/runinfo.txt)
Uploads will be available until `2016-09-17 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67572107
--- Diff: server/src/com/cloud/vm/UserVmManagerImpl.java ---
@@ -2086,6 +2089,18 @@ public boolean expunge(UserVmVO vm, long callerUserId, Account caller) {
}
}
+ /**
+ * Release network resources, it was done on vm stop previously.
+ * @param id vm id
+ * @throws ConcurrentOperationException
+ * @throws ResourceUnavailableException
+ */
+ private void releaseNetworkResourcesOnExpunge(long id) throws ConcurrentOperationException, ResourceUnavailableException {
+ final VMInstanceVO vmInstance = _vmDao.findById(id);
--- End diff --
It is possible for ``_vmDao.findById`` to return ``null``. Please handling for that case.
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1594
---
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.
---
[GitHub] cloudstack issue #1594: CLOUDSTACK-9407: vm_network_map table doesnt get cle...
Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on the issue:
https://github.com/apache/cloudstack/pull/1594
Thanks @jburwell @swill for reviewing!
I refactored this PR based on @jburwell comments, it would be nice if it can be included!
---
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.
---
[GitHub] cloudstack issue #1594: CLOUDSTACK-9407: vm_network_map table doesnt get cle...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the issue:
https://github.com/apache/cloudstack/pull/1594
@jburwell I will do testing on this and if everything comes back clean we can probably add it. It is not a huge issue, but bug fixes are worth getting in IMO.
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67571985
--- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
@@ -1577,8 +1577,9 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
}
try {
- _networkMgr.release(profile, cleanUpEvenIfUnableToStop);
- s_logger.debug("Successfully released network resources for the vm " + vm);
+ s_logger.debug("Not releasing network resources until expunge command is sent");
+ //_networkMgr.release(profile, cleanUpEvenIfUnableToStop);
+ //s_logger.debug("Successfully released network resources for the vm " + vm);
--- End diff --
Please remove commented code. It is cruft, and git history allows us to see how code has changed overtime.
---
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.
---
[GitHub] cloudstack issue #1594: CLOUDSTACK-9407: vm_network_map table doesnt get cle...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on the issue:
https://github.com/apache/cloudstack/pull/1594
@swill I noticed that the ticket was opened after the 4.9.0 code freeze, but it is targeted to 4.9.0. Is it severe enough to be included in 4.9.0 or should it deferred to 4.9.1/4.10?
---
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.
---
[GitHub] cloudstack pull request #1594: CLOUDSTACK-9407: vm_network_map table doesnt ...
Posted by nvazquez <gi...@git.apache.org>.
Github user nvazquez commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1594#discussion_r67907402
--- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
@@ -1577,8 +1577,9 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
}
try {
- _networkMgr.release(profile, cleanUpEvenIfUnableToStop);
- s_logger.debug("Successfully released network resources for the vm " + vm);
+ s_logger.debug("Not releasing network resources until expunge command is sent");
--- End diff --
I decided to remove try-block as it only logged info, and include it on method's javadoc, as I don't think it could be worth logging. Do you agree?
---
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.
---