Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910613418 > > changing vm_memballoon_stats_period to 60, will add a new setting in the vm definition, which will probably cause some regression which is out of our control. We should disable it by default. > > But again, it is a speculation. We have solid use cases of operators having to contact the community to understand why their Windows VMs are with wrong stats; and every time, the solution is to change the `agent.properties`. However, we do not have use cases that corroborate with the situation you said you faced a long time ago. Indeed, we do have use cases that shows that the aforementioned situation does not happen, vide https://github.com/apache/cloudstack/issues/8453#issuecomment-1881793055. Therefore, we do not have a solid use case/reason to not change the value. > > Please, bring solid use cases so we can discuss it further. interesting Your use case is solid my issue is not solid -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
GutoVeronezi commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910586343 > changing vm_memballoon_stats_period to 60, will add a new setting in the vm definition, which will probably cause some regression which is out of our control. We should disable it by default. But again, it is a speculation. We have solid use cases of operators having to contact the community to understand why their Windows VMs are with wrong stats; and every time, the solution is to change the `agent.properties`. However, we do not have use cases that corroborate with the situation you said you faced a long time ago. Indeed, we do have use cases that shows that the aforementioned situation does not happen, vide https://github.com/apache/cloudstack/issues/8453#issuecomment-1881793055. Therefore, we do not have a solid use case/reason to not change the value. Please, bring solid use cases so we can discuss it further. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910464449 > > @GutoVeronezi you know some issues are difficult to reproduce. the issue could be related to the template (Windows version, virtio drivers, etc), and the host (Linux OS version, qemu/libvirt version), etc I did not face any issue with Linux VMs caused by it. so I made some changes to enable it on Linux VMs, but disable it for Windows VMs. > > But then we do not have a solid reason to not change the value, just speculations. If at least we could reproduce the error you saying that exist, we could look for the real cause and solve the problem (or at least find an explanation). @GutoVeronezi If you are running a production, you might know, when we face an issue (Windows VM got frozen after live migration in my case), the first thing we need to do, is to find the workaround which fix the issue and then avoid it (in my case, the workaround is disabling memory stats). root cause? in my case, we need to dive into the source code of QEMU and virtio driver. the root cause is not important to me, as I can accept the problem that Windows VM has wrong memory usage (100%) which is a minor issue, espscially comparing with the issue that Windows VM got frozen. changing vm_memballoon_stats_period to 60, will add a new setting in the vm definition, which will probably cause some regression which is out of our control. We should disable it by default. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
GutoVeronezi commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910418806 > @GutoVeronezi you know some issues are difficult to reproduce. the issue could be related to the template (Windows version, virtio drivers, etc), and the host (Linux OS version, qemu/libvirt version), etc I did not face any issue with Linux VMs caused by it. so I made some changes to enable it on Linux VMs, but disable it for Windows VMs. But then we do not have a solid reason to not change the value, just speculations. If at least we could reproduce the error you saying that exist, we could look for the real cause and solve the problem (or at least find an explanation). -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910397218 > > @GutoVeronezi I have mentioned this `but I am sure it was caused by memory stat, because the issue is gone after disabling memory stat on Windows vms.` > > Anyone who wants to enable the memory stats collection and happy to take the risky, go for it. ACS has already provided the option to do it. > > @weizhouapache do you have steps to reproduce it? @GutoVeronezi you know some issues are difficult to reproduce. the issue could be related to the template (Windows version, virtio drivers, etc), and the host (Linux OS version, qemu/libvirt version), etc I did not face any issue with Linux VMs caused by it. so I made some changes to enable it on Linux VMs, but disable it for Windows VMs. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
GutoVeronezi commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910375522 > @GutoVeronezi I have mentioned this `but I am sure it was caused by memory stat, because the issue is gone after disabling memory stat on Windows vms.` > > Anyone who wants to enable the memory stats collection and happy to take the risky, go for it. ACS has already provided the option to do it. @weizhouapache do you have steps to reproduce it? -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910372571 > > can please see my comments above ? @GutoVeronezi > > @weizhouapache you said you have faced an issue in the past, but did not specify how to reproduce it or its details. On the other hand, you said that you are not sure if the issue was caused by the virtio drivers; therefore, we do not have a solid reason to not change it. @GutoVeronezi I have mentioned this `but I am sure it was caused by memory stat, because the issue is gone after disabling memory stat on Windows vms.` Anyone who wants to enable the memory stats collection and happy to take the risky, go for it. ACS has already provided the option to do it. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
GutoVeronezi commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910336921 > can please see my comments above ? @GutoVeronezi @weizhouapache you said you have faced an issue in the past, but did not specify how to reproduce it or its details. On the other hand, you said that you are not sure if the issue was caused by the virtio drivers; therefore, we do not have a solid reason to not change it. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1910257919 > hat are your concerns with this change? Without it, a nice feature in ACS, that enables users to monitor users' VMs via UI does not work out of the box for KVM deployments, which can cause disappointments and misunderstandings as the ones already noticed in the mailing list and issues in Github. can please see my comments above ? @GutoVeronezi -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
GutoVeronezi commented on code in PR #8520:
URL: https://github.com/apache/cloudstack/pull/8520#discussion_r1466253577
##
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##
@@ -724,9 +724,9 @@ public Property getWorkers() {
/**
* The time interval (in seconds) at which the balloon driver will get
memory stats updates. This is equivalent to Libvirt's --period
parameter when using the dommemstat command.
* Data type: Integer.
- * Default value: 0
+ * Default value: 60
*/
-public static final Property VM_MEMBALLOON_STATS_PERIOD = new
Property<>("vm.memballoon.stats.period", 0);
+public static final Property VM_MEMBALLOON_STATS_PERIOD = new
Property<>("vm.memballoon.stats.period", 60);
Review Comment:
File `agent.properties` must be changed as well.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
weizhouapache commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1898632361 > I believe that metrics monitoring should work without operators having to worry about configurations and enabling/disabling the feature. Changing the configuration value from '60' to '0' compromised the functionality for the KVM hypervisor; which in turn, can make it seem as it does not work out of the box. An example of this is operators lacking visibility regarding the necessity of adding this configuration in the agent.properties file (see: [#8453 (comment)](https://github.com/apache/cloudstack/issues/8453#issuecomment-1880120221)). > In my opinion, the wrong memory usage is not a critical issue for operators. I believe most cloud operators use automation tools (chef, ansible, puppet, etc). It is very easy to apply the changes on all kvm hosts. > From my perspective, using the value '60' resolves issues similar to those faced by @shaerul in #8453. What are the main impacts you foresee with this change? As I said, I have faced an issue in the past. I am not sure if the issue was caused by windows virtio drivers or qemu/libvirt. anyway, we do not need to make decision for users. There is already a configuration for users to enable/disable it or set to a different value. > > Regarding the issue encountered during the migration of a Windows instance with the memory balloon enabled ([#8453 (comment)](https://github.com/apache/cloudstack/issues/8453#issuecomment-1880956758)), could you explain the error scenario in detail? This would allow me to attempt to reproduce locally and, if necessary, extend this PR to address the issue related to the migration of Windows VMs with the memory balloon feature enabled. I do not remember the details very clearly. the kvm host is Ubuntu 18 or 20 the issue was difficult to reproduce, it happened to some vms with virtio drivers, large memory and memory intensive applications. but I am sure it was caused by memory stat, because the issue is gone after disabling memory stat on Windows vms. cc @RodrigoDLopez -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Changes the value of config vm_memballoon_stats_period to 60 [cloudstack]
RodrigoDLopez commented on PR #8520: URL: https://github.com/apache/cloudstack/pull/8520#issuecomment-1898571269 I believe that metrics monitoring should work without operators having to worry about configurations and enabling/disabling the feature. Changing the configuration value from '60' to '0' compromised the functionality for the KVM hypervisor; which in turn, can make it seem as it does not work out of the box. An example of this is operators lacking visibility regarding the necessity of adding this configuration in the agent.properties file (see: https://github.com/apache/cloudstack/issues/8453#issuecomment-1880120221). From my perspective, using the value '60' resolves issues similar to those faced by @shaerul in #8453. What are the main impacts you foresee with this change? --- Regarding the issue encountered during the migration of a Windows instance with the memory balloon enabled (https://github.com/apache/cloudstack/issues/8453#issuecomment-1880956758), could you explain the error scenario in detail? This would allow me to attempt to reproduce locally and, if necessary, extend this PR to address the issue related to the migration of Windows VMs with the memory balloon feature enabled. -- 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-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
