You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/10/27 11:47:15 UTC
[GitHub] [cloudstack] joseflauzino opened a new pull request #5633: Fix metrics stats for VMs not running
joseflauzino opened a new pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633
### Description
This PR fixes the behavior of the stats collector with respect to VMs that are not running. The need for this is that if a running VM has a state change for some reason, ACS keeps showing the latest metrics stats collected.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### How Has This Been Tested?
Using ACS UI, I performed the following steps:
- I deployed a VM and waited for the complete boot;
- I enabled the presentation of metric statistics in the UI;
- I waited for ACS to collect and show metrics stats;
- And then, I tested two different cases:
- Case 1 - I stopped VM using ACS UI;
- Case 2 - I simulated a VM crash by manually stopping it (directly from the hypervisor);
- In both cases, the metrics stats for the VM were successfully cleared. As expected, when I tested Case 1, cleanup was done immediately. In Case 2, I had to wait for ACS to identify that the VM was stopped and the stats collector try to collect new metric stats.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956682299
<b>Trillian test result (tid-2502)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31865 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5633-t2502-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972930060
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 1739
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956682299
<b>Trillian test result (tid-2502)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31865 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5633-t2502-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972665408
code looks good, let's retest
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r741161031
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
Thanks for the review @sureshanaparti!
The `cleanUpVirtualMachineStats()` method is important to maintain consistency of the metric stats shown by ACS even when there are cases of VMs changing state unexpectedly (e.g., when a VM crashes; or when the VM is stopped directly by the hypervisor interface, etc). In these cases the `removeVirtualMachineStats()` method is not executed, so the `cleanUpVirtualMachineStats()` method does its job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-974036010
@joseflauzino a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956197237
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-953773323
@sureshanaparti and @DaanHoogland
Thanks for the suggestions!
Because `removeAll()` returns a boolean, I had to break into two lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-986803804
@sureshanaparti for me we can keep it in the main (4.17).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956099312
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972869678
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 1734
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956143129
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1646
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-986806973
Merging this based on 2 LGTMs and [package](https://github.com/apache/cloudstack/pull/5633#issuecomment-974081799) and [test](https://github.com/apache/cloudstack/pull/5633#issuecomment-975810322) results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956196947
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-986747193
@joseflauzino Is this good to go in 4.16.1? If so, please rebase with '4.16'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-974035322
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-970281134
Any update about this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r741161031
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
Thanks for the review @sureshanaparti!
The `cleanUpVirtualMachineStats()` method is important to maintain consistency of the metric stats shown by ACS even when there are cases of VMs changing state unexpectedly (e.g., when a VM crashes; or when the VM is stopped directly by the hypervisor interface, etc). In these cases the `removeVirtualMachineStats()` method is not executed, so the `cleanUpVirtualMachineStats()` method does its job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r741161031
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
Thanks for the review @sureshanaparti!
The `cleanUpVirtualMachineStats()` method is important to maintain consistency of the metric stats shown by ACS even when there are cases of VMs changing state unexpectedly (e.g., when a VM crashes; or when the VM is stopped directly by the hypervisor interface, etc). In these cases the `removeVirtualMachineStats()` method is not executed, so the `cleanUpVirtualMachineStats()` method does its job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r737508016
##########
File path: server/src/test/java/com/cloud/server/StatsCollectorTest.java
##########
@@ -218,4 +239,48 @@ public void configureAndTestCheckIfDiskStatsAreZero(long bytesRead, long bytesWr
boolean result = statsCollector.areAllDiskStatsZero(vmDiskStatsEntry);
Assert.assertEquals(expected, result);
}
+
+ @Test
+ public void removeVirtualMachineStatsTestRemoveOneVmStats() {
+ Mockito.doReturn(new Object()).when(vmStatsMock).remove(Mockito.anyLong());
+
+ statsCollector.removeVirtualMachineStats(1l);
+
+ Mockito.verify(vmStatsMock, Mockito.times(1)).remove(Mockito.anyLong());
+ }
+
+ @Test
+ public void cleanUpVirtualMachineStatsTestDoNothing() {
+ Mockito.doReturn(new ArrayList<>()).when(userVmDaoMock).listAllRunning();
+ Mockito.doReturn(new ConcurrentHashMap<Long, VmStats>(new HashMap<Long, VmStats>()).keySet())
Review comment:
```suggestion
Mockito.doReturn(new ConcurrentHashMap<Long, VmStats>(new HashMap<>()).keySet())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-974081799
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1749
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino edited a comment on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
joseflauzino edited a comment on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-953773323
@sureshanaparti and @DaanHoogland
Done.
Thanks for the suggestions!
Because `removeAll()` returns a boolean, I had to break into two lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-975328222
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-975810322
<b>Trillian test result (tid-2572)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31220 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5633-t2572-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956682299
<b>Trillian test result (tid-2502)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31865 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5633-t2502-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-986691819
@GutoVeronezi yes, I think @sureshanaparti's question was answered to satisfaction as well. let's merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi merged pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
GutoVeronezi merged pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r737507119
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,36 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
+ allRunningVmIds.add(vm.getId());
+ }
+
+ List<Long> vmIdsToRemoveStats = new ArrayList<Long>();
Review comment:
or
```suggestion
List<Long> vmIdsToRemoveStats = new ArrayList<>(_VmStats.keySet())).removeAll(allRunningVmIds);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-956101126
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r740997586
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
VM stats are captured for running VMs only
https://github.com/apache/cloudstack/blob/a78af608144f80649c409ebd933265d2d755f289/server/src/main/java/com/cloud/server/StatsCollector.java#L579
_removeVirtualMachineStats()_ removes the vm stat if the VM is stopped/destroyed after adding to the stats, so no need for this cleanup method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972665827
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-975327464
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972871871
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#issuecomment-972870917
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r737400295
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,36 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
+ allRunningVmIds.add(vm.getId());
+ }
+
+ List<Long> vmIdsToRemoveStats = new ArrayList<Long>();
Review comment:
```suggestion
List<Long> vmIdsToRemoveStats = (new ArrayList< Long >(_VmStats.keySet())).removeAll(allRunningVmIds);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r740997586
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
VM stats are captured for running VMs only
https://github.com/apache/cloudstack/blob/a78af608144f80649c409ebd933265d2d755f289/server/src/main/java/com/cloud/server/StatsCollector.java#L579
_removeVirtualMachineStats()_ removes the vm stat if the VM is stopped/destroyed after adding to the stats, so no need for this cleanup method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #5633: Fix metrics stats for VMs not running
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5633:
URL: https://github.com/apache/cloudstack/pull/5633#discussion_r740997586
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -1485,6 +1487,32 @@ private void storeVirtualMachineStatsInMemory(VmStatsEntry statsForCurrentIterat
}
}
+ /**
+ * Removes stats for a given virtual machine.
+ * @param vmId ID of the virtual machine to remove stats.
+ */
+ public void removeVirtualMachineStats(Long vmId) {
+ s_logger.debug(String.format("Removing stats from VM with ID: %s .",vmId));
+ _VmStats.remove(vmId);
+ }
+
+ /**
+ * Removes stats of virtual machines that are not running from memory.
+ */
+ protected void cleanUpVirtualMachineStats() {
+ List<Long> allRunningVmIds = new ArrayList<Long>();
+ for (UserVmVO vm : _userVmDao.listAllRunning()) {
Review comment:
VM stats are captured for running VMs only
https://github.com/apache/cloudstack/blob/a78af608144f80649c409ebd933265d2d755f289/server/src/main/java/com/cloud/server/StatsCollector.java#L579
_removeVirtualMachineStats()_ removes the vm stat if the VM is stopped/destroyed after adding to the stats, so no need for this cleanup method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org