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/01/20 10:12:08 UTC

[GitHub] [cloudstack] ustcweizhou opened a new pull request #4601: CLSTACK-3660: Get vm network/disk statistics and update database per host

ustcweizhou opened a new pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601


   ### Description
   
   This PR fixes an issue when collect vm network or disk statistics from hosts.
   
   If cloudstack fails to get vm statistics from a host, the whole process will be terminated, and it will not get vm statistics from remaining hosts.
   
   With this pr, the host with problem will be ignored, and cloudstack will continue on remaining hosts.
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### 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
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [X] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766732816






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767338623


   @shwstppr 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767326704


   Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2591


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766753689


   Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2587


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-771658707


   3 checksum checks that failed on vmware. not related but very disturbing!?!


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767338268


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766860780


   @DaanHoogland thanks for review. modified some debug message
   
   @shwstppr code is changed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766732578


   @ustcweizhou do you plan to address above comments?
   
   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-771658707


   3 checksum checks that failed on vmware. not related but very disturbing!?!


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766732578


   @ustcweizhou do you plan to address above comments?
   
   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767830197


   <b>Trillian test result (tid-3414)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42587 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4601-t3414-vmware-67u3.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_templates.py
   Smoke tests completed. 81 look OK, 2 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 252.62 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3612.82 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 120.22 | test_kubernetes_clusters.py
   test_02_create_template_with_checksum_sha1 | `Error` | 5.11 | test_templates.py
   test_03_create_template_with_checksum_sha256 | `Error` | 5.11 | test_templates.py
   test_04_create_template_with_checksum_md5 | `Error` | 5.10 | test_templates.py
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766732816


   @shwstppr 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767727354


   <b>Trillian test result (tid-3412)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39607 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4601-t3412-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Smoke tests completed. 82 look OK, 1 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 3615.86 | test_kubernetes_clusters.py
   test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 103.09 | test_kubernetes_clusters.py
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767317125


   @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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767399129


   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766860780


   @DaanHoogland thanks for review. modified some debug message
   
   @shwstppr code is changed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#discussion_r561831461



##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -793,10 +792,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                 }
                             }
                         }
-                    }
-                });
-            } catch (Exception e) {
-                s_logger.warn("Error while collecting vm disk stats from hosts", e);
+                    });
+                } catch (Exception e) {
+                    s_logger.warn("Error while collecting vm disk stats from hosts", e);

Review comment:
       we could add host details in this message

##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -684,18 +684,17 @@ protected void runInContext() {
                 return;
             }
             // collect the vm disk statistics(total) from hypervisor. added by weizhou, 2013.03.
-            s_logger.trace("Running VM disk stats ...");
-            try {
-                Transaction.execute(new TransactionCallbackNoReturn() {
-                    @Override
-                    public void doInTransactionWithoutResult(TransactionStatus status) {
-                        s_logger.debug("VmDiskStatsTask is running...");
+            s_logger.debug("VmDiskStatsTask is running...");

Review comment:
       why, this change in message and level?

##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -912,10 +911,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                 }
                             }
                         }
-                    }
-                });
-            } catch (Exception e) {
-                s_logger.warn("Error while collecting vm network stats from hosts", e);
+                    });
+                } catch (Exception e) {
+                    s_logger.warn("Error while collecting vm network stats from hosts", e);

Review comment:
       again, we can now add host details to the message




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd merged pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] ustcweizhou closed pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
ustcweizhou closed pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#discussion_r563773324



##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -684,18 +684,17 @@ protected void runInContext() {
                 return;
             }
             // collect the vm disk statistics(total) from hypervisor. added by weizhou, 2013.03.
-            s_logger.trace("Running VM disk stats ...");
-            try {
-                Transaction.execute(new TransactionCallbackNoReturn() {
-                    @Override
-                    public void doInTransactionWithoutResult(TransactionStatus status) {
-                        s_logger.debug("VmDiskStatsTask is running...");
+            s_logger.debug("VmDiskStatsTask is running...");

Review comment:
       @DaanHoogland the message and level are not changed.  moved out from try block.
   
   Removed the line which is not needed as we have debug message.
   `s_logger.trace("Running VM disk 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#discussion_r561831461



##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -793,10 +792,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                 }
                             }
                         }
-                    }
-                });
-            } catch (Exception e) {
-                s_logger.warn("Error while collecting vm disk stats from hosts", e);
+                    });
+                } catch (Exception e) {
+                    s_logger.warn("Error while collecting vm disk stats from hosts", e);

Review comment:
       we could add host details in this message

##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -684,18 +684,17 @@ protected void runInContext() {
                 return;
             }
             // collect the vm disk statistics(total) from hypervisor. added by weizhou, 2013.03.
-            s_logger.trace("Running VM disk stats ...");
-            try {
-                Transaction.execute(new TransactionCallbackNoReturn() {
-                    @Override
-                    public void doInTransactionWithoutResult(TransactionStatus status) {
-                        s_logger.debug("VmDiskStatsTask is running...");
+            s_logger.debug("VmDiskStatsTask is running...");

Review comment:
       why, this change in message and level?

##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -912,10 +911,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                 }
                             }
                         }
-                    }
-                });
-            } catch (Exception e) {
-                s_logger.warn("Error while collecting vm network stats from hosts", e);
+                    });
+                } catch (Exception e) {
+                    s_logger.warn("Error while collecting vm network stats from hosts", e);

Review comment:
       again, we can now add host details to the message




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767317405


   @shwstppr 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-766732816


   @shwstppr 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#issuecomment-767398485


   This may require testing on different hypervisors too
   @blueorangutan test centos7 vmware-67u3 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4601: server: Get vm network/disk statistics and update database per host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4601:
URL: https://github.com/apache/cloudstack/pull/4601#discussion_r563773324



##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -684,18 +684,17 @@ protected void runInContext() {
                 return;
             }
             // collect the vm disk statistics(total) from hypervisor. added by weizhou, 2013.03.
-            s_logger.trace("Running VM disk stats ...");
-            try {
-                Transaction.execute(new TransactionCallbackNoReturn() {
-                    @Override
-                    public void doInTransactionWithoutResult(TransactionStatus status) {
-                        s_logger.debug("VmDiskStatsTask is running...");
+            s_logger.debug("VmDiskStatsTask is running...");

Review comment:
       @DaanHoogland the message and level are not changed.  moved out from try block.
   
   Removed the line which is not needed as we have debug message.
   `s_logger.trace("Running VM disk 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org