You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by ustcweizhou <gi...@git.apache.org> on 2015/10/13 16:44:34 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-7984: Collect network statisti...

GitHub user ustcweizhou opened a pull request:

    https://github.com/apache/cloudstack/pull/926

    CLOUDSTACK-7984: Collect network statistics for VMs on shared network (KVM implementation)

    For now we can get network traffic statistics from virtual router for isolated network.
    For nics on shared networks, we need to collect the traffic as well. 
    It is useful for utility billing, especially for basic zone or advanced zone with security groups.
    This supports KVM until now.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ustcweizhou/cloudstack CLOUDSTACK-7984

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/926.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 #926
    
----
commit ec615bb794c0fccdc5900609ceaee91248371cb4
Author: Wei Zhou <w....@tech.leaseweb.com>
Date:   2015-10-12T12:22:29Z

    CLOUDSTACK-7984: Collect network statistics for VMs on shared network (KVM implementation)

----


---
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: [4.7] CLOUDSTACK-7984: Collect network st...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-158676063
  
    @wido I see some comments above. Not sure if they are all addressed. Can you please review this and let us know if you are OK with merging? I'll run soms tests again since it's been a while.


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/926#discussion_r41961770
  
    --- Diff: server/src/com/cloud/server/StatsCollector.java ---
    @@ -310,8 +326,17 @@ private void init(Map<String, String> configs) {
                 _executor.scheduleAtFixedRate(new VmDiskStatsTask(), vmDiskStatsInterval, vmDiskStatsInterval, TimeUnit.SECONDS);
             }
     
    +        if (vmNetworkStatsInterval > 0) {
    +            if (vmNetworkStatsInterval < 300)
    +                vmNetworkStatsInterval = 300;
    +            _executor.scheduleAtFixedRate(new VmNetworkStatsTask(), vmNetworkStatsInterval, vmNetworkStatsInterval, TimeUnit.SECONDS);
    +        } else {
    +            s_logger.debug("vm.network.stats.interval - " + vmNetworkStatsInterval + " so not scheduling the vm network stats thread");
    --- End diff --
    
    Maybe explain here that a negative value is not allowed?


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-150531546
  
    Talked to @DaanHoogland and he wants this in 4.6. It seems like a feature and not a bug fix, but pinging @karuturi to align.


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-149947059
  
    LGTM basic smoke test run failed on the load balancer tests only:
    
    ----------------------------------------------------------------------
    Ran 40 tests in 7108.901s
    
    FAILED (errors=3)



---
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: [4.7] CLOUDSTACK-7984: Collect network st...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-175672456
  
    @ustcweizhou Can you please rebase against latest master


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-147969907
  
    @wido the default value is 0, which means the statistics is disabled by default.
    I will remove the 300 seconds limitation if most of us agree.
    the vm disk statistics has the same setting to vm network statistics.
    
    



---
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: [4.7] CLOUDSTACK-7984: Collect network st...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-216196724
  
    @ustcweizhou please rebase against latest master and push -f, update on status of your PR; also squash changes to a single commit
    
    tag:easypr


---
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 #926: [4.10] CLOUDSTACK-7984: Collect network statistics fo...

Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:

    https://github.com/apache/cloudstack/pull/926
  
    @blueorangutan package


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-152306229
  
    ran 
    ```
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \
    component/test_vpc_redundant.py \
    component/test_routers_iptables_default_policy.py \
    component/test_routers_network_ops.py \
    component/test_vpc_router_nics.py \
    smoke/test_loadbalance.py \
    smoke/test_internal_lb.py \
    smoke/test_ssvm.py \
    smoke/test_network.py
    ```
    resulting in
    ```
    Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : SUCCESS ===
    ok
    Create a redundant VPC with two networks with two VMs in each network and check default routes ... === TestName: test_02_redundant_VPC_default_routes | Status : SUCCESS ===
    ok
    Test iptables default INPUT/FORWARD policy on RouterVM ... === TestName: test_02_routervm_iptables_policies | Status : SUCCESS ===
    ok
    Test iptables default INPUT/FORWARD policies on VPC router ... === TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
    ok
    Stop existing router, add a PF rule and check we can access the VM ... === TestName: test_isolate_network_FW_PF_default_routes | Status : SUCCESS ===
    ok
    Test redundant router internals ... === TestName: test_RVR_Network_FW_PF_SSH_default_routes | Status : SUCCESS ===
    ok
    Create a VPC with two networks with one VM in each network and test nics after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : SUCCESS ===
    ok
    Create a VPC with two networks with one VM in each network and test default routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
    ok
    Test to create Load balancing rule with source NAT ... === TestName: test_01_create_lb_rule_src_nat | Status : SUCCESS ===
    ok
    Test to create Load balancing rule with non source NAT ... === TestName: test_02_create_lb_rule_non_nat | Status : SUCCESS ===
    ok
    Test for assign & removing load balancing rule ... === TestName: test_assign_and_removal_lb | Status : SUCCESS ===
    ok
    Test to verify access to loadbalancer haproxy admin stats page ... === TestName: test02_internallb_haproxy_stats_on_all_interfaces | Status : SUCCESS ===
    ok
    Test create, assign, remove of an Internal LB with roundrobin http traffic to 3 vm's ... === TestName: test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
    ok
    Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : SUCCESS ===
    ok
    Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : SUCCESS ===
    ok
    Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
    ok
    Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
    ok
    Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS ===
    ok
    Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS ===
    ok
    Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS ===
    ok
    Test destroy CPVM ... === TestName: test_10_destroy_cpvm | Status : SUCCESS ===
    ok
    Test for port forwarding on source NAT ... === TestName: test_01_port_fwd_on_src_nat | Status : SUCCESS ===
    ok
    Test for port forwarding on non source NAT ... === TestName: test_02_port_fwd_on_non_src_nat | Status : SUCCESS ===
    ok
    Test for reboot router ... === TestName: test_reboot_router | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_1_static_nat_rule | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_2_nat_rule | Status : SUCCESS ===
    ok
    Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 27 tests in 12295.891s
    ```
    and
    ```
    nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=false \
    smoke/test_routers.py \
    smoke/test_network_acl.py \
    smoke/test_privategw_acl.py \
    smoke/test_reset_vm_on_reboot.py \
    smoke/test_vm_life_cycle.py \
    smoke/test_vpc_vpn.py \
    smoke/test_service_offerings.py \
    smoke/test_network.py \
    component/test_vpc_offerings.py \
    component/test_vpc_routers.py
    ```
    resulting in
    ```
    Test router internal advanced zone ... === TestName: test_02_router_internal_adv | Status : SUCCESS ===
    ok
    Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
    ok
    Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
    ok
    Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
    ok
    Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
    ok
    Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
    ok
    Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
    ok
    test_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_privategw_acl | Status : SUCCESS ===
    ok
    Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS ===
    ok
    Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
    ok
    Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
    ok
    Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
    ok
    Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
    ok
    Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
    ok
    Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
    ok
    Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
    ok
    Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
    ok
    Test migrate VM ... SKIP: At least two hosts should be present in the zone for migration
    Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
    ok
    Test Remote Access VPN in VPC ... === TestName: test_vpc_remote_access_vpn | Status : SUCCESS ===
    ok
    Test VPN in VPC ... === TestName: test_vpc_site2site_vpn | Status : SUCCESS ===
    ok
    Test to create service offering ... === TestName: test_01_create_service_offering | Status : SUCCESS ===
    ok
    Test to update existing service offering ... === TestName: test_02_edit_service_offering | Status : SUCCESS ===
    ok
    Test to delete service offering ... === TestName: test_03_delete_service_offering | Status : SUCCESS ===
    ok
    Test for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
    ok
    Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
    ok
    Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
    ok
    Test for release public IP address ... === TestName: test_releaseIP | Status : SUCCESS ===
    ok
    Test create VPC offering ... === TestName: test_01_create_vpc_offering | Status : SUCCESS ===
    ok
    Test VPC offering without load balancing service ... === TestName: test_03_vpc_off_without_lb | Status : SUCCESS ===
    ok
    Test VPC offering without static NAT service ... === TestName: test_04_vpc_off_without_static_nat | Status : SUCCESS ===
    ok
    Test VPC offering without port forwarding service ... === TestName: test_05_vpc_off_without_pf | Status : SUCCESS ===
    ok
    Test VPC offering with invalid services ... === TestName: test_06_vpc_off_invalid_services | Status : SUCCESS ===
    ok
    Test update VPC offering ... === TestName: test_07_update_vpc_off | Status : SUCCESS ===
    ok
    Test list VPC offering ... === TestName: test_08_list_vpc_off | Status : SUCCESS ===
    ok
    test_09_create_redundant_vpc_offering (integration.component.test_vpc_offerings.TestVPCOffering) ... === TestName: test_09_create_redundant_vpc_offering | Status : SUCCESS ===
    ok
    Test start/stop of router after addition of one guest network ... === TestName: test_01_start_stop_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test reboot of router after addition of one guest network ... === TestName: test_02_reboot_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to change service offering of router after addition of one guest network ... === TestName: test_04_chg_srv_off_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test destroy of router after addition of one guest network ... === TestName: test_05_destroy_router_after_addition_of_one_guest_network | Status : SUCCESS ===
    ok
    Test to stop and start router after creation of VPC ... === TestName: test_01_stop_start_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Test to reboot the router after creating a VPC ... === TestName: test_02_reboot_router_after_creating_vpc | Status : SUCCESS ===
    ok
    Tests to change service offering of the Router after ... === TestName: test_04_change_service_offerring_vpc | Status : SUCCESS ===
    ok
    Test to destroy the router after creating a VPC ... === TestName: test_05_destroy_router_after_creating_vpc | Status : SUCCESS ===
    ok
    
    ----------------------------------------------------------------------
    Ran 44 tests in 9112.236s
    ```
    
    lgtm based on trust


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-147962383
  
    @ustcweizhou I understand the reasoning behind the 300 second minimal interval, but isn't that up to the admin? If he/she wants to set it to 10 seconds on their cloud, maybe for testing purposes, let them.
    
    We should set it to a sane default value, that's all.


---
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 #926: [4.10] CLOUDSTACK-7984: Collect network statistics fo...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/926
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-527


---
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 #926: [4.10] CLOUDSTACK-7984: Collect network statistics fo...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/926
  
    @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-150911520
  
    rerunning the ssvm and vpc tests 


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-153688875
  
    LGTM (just code, no testing performed)


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-147961785
  
    @wido I will add some debug/info log messages later.
    Do you think if it is good to remove the hardcoded 300 seconds?


---
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: [4.7] CLOUDSTACK-7984: Collect network st...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-180467220
  
    rebased


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-150900227
  
    @remibergsma code is updated


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/926#issuecomment-150836618
  
    @ustcweizhou Can you please rebase with current master so I can test this? 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 #926: [4.10] CLOUDSTACK-7984: Collect network statistics fo...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/926
  
    rebased with latest master


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/926#discussion_r41961743
  
    --- Diff: server/src/com/cloud/server/StatsCollector.java ---
    @@ -310,8 +326,17 @@ private void init(Map<String, String> configs) {
                 _executor.scheduleAtFixedRate(new VmDiskStatsTask(), vmDiskStatsInterval, vmDiskStatsInterval, TimeUnit.SECONDS);
             }
     
    +        if (vmNetworkStatsInterval > 0) {
    +            if (vmNetworkStatsInterval < 300)
    --- End diff --
    
    So why is this hardcoded to be at least 300? Shouldn't we at least throw a info log message here telling the admin that we do this?


---
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: CLOUDSTACK-7984: Collect network statisti...

Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/926#discussion_r41961830
  
    --- Diff: server/src/com/cloud/server/StatsCollector.java ---
    @@ -700,6 +734,121 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
             }
         }
     
    +    class VmNetworkStatsTask extends ManagedContextRunnable {
    +        @Override
    +        protected void runInContext() {
    +            //Check for ownership
    +            //msHost in UP state with min id should run the job
    +            ManagementServerHostVO msHost = _msHostDao.findOneInUpState(new Filter(ManagementServerHostVO.class, "id", true, 0L, 1L));
    +            if(msHost == null || (msHost.getMsid() != mgmtSrvrId)){
    +                s_logger.debug("Skipping collect vm network stats from hosts");
    +                return;
    +            }
    +            // collect the vm network statistics(total) from hypervisor
    +            try {
    +                Transaction.execute(new TransactionCallbackNoReturn() {
    +                    @Override
    +                    public void doInTransactionWithoutResult(TransactionStatus status) {
    +                        s_logger.debug("VmNetworkStatsTask is running...");
    +
    +                        SearchCriteria<HostVO> sc = _hostDao.createSearchCriteria();
    +                        sc.addAnd("status", SearchCriteria.Op.EQ, Status.Up.toString());
    +                        sc.addAnd("resourceState", SearchCriteria.Op.NIN, ResourceState.Maintenance, ResourceState.PrepareForMaintenance, ResourceState.ErrorInMaintenance);
    +                        sc.addAnd("type", SearchCriteria.Op.EQ, Host.Type.Routing.toString());
    +                        List<HostVO> hosts = _hostDao.search(sc, null);
    +
    +                        for (HostVO host : hosts)
    +                        {
    +                            List<UserVmVO> vms = _userVmDao.listRunningByHostId(host.getId());
    +                            List<Long> vmIds = new ArrayList<Long>();
    +
    +                            for (UserVmVO vm : vms) {
    +                                if (vm.getType() == VirtualMachine.Type.User) // user vm
    +                                    vmIds.add(vm.getId());
    +                            }
    +
    +                            HashMap<Long, List<VmNetworkStatsEntry>> vmNetworkStatsById = _userVmMgr.getVmNetworkStatistics(host.getId(), host.getName(), vmIds);
    +                            if (vmNetworkStatsById == null)
    +                                continue;
    +
    +                            Set<Long> vmIdSet = vmNetworkStatsById.keySet();
    +                            for(Long vmId : vmIdSet)
    +                            {
    +                                List<VmNetworkStatsEntry> vmNetworkStats = vmNetworkStatsById.get(vmId);
    +                                if (vmNetworkStats == null)
    +                                    continue;
    +                                UserVmVO userVm = _userVmDao.findById(vmId);
    +                                for (VmNetworkStatsEntry vmNetworkStat:vmNetworkStats) {
    +                                    SearchCriteria<NicVO> sc_nic = _nicDao.createSearchCriteria();
    +                                    sc_nic.addAnd("macAddress", SearchCriteria.Op.EQ, vmNetworkStat.getMacAddress());
    +                                    NicVO nic = _nicDao.search(sc_nic, null).get(0);
    +                                    List<VlanVO> vlan = _vlanDao.listVlansByNetworkId(nic.getNetworkId());
    +                                    if (vlan == null || vlan.size() == 0 || vlan.get(0).getVlanType() != VlanType.DirectAttached)
    +                                        continue; // only get network statistics for DirectAttached network (shared networks in Basic zone and Advanced zone with/without SG)
    +                                    UserStatisticsVO previousvmNetworkStats = _userStatsDao.findBy(userVm.getAccountId(), userVm.getDataCenterId(), nic.getNetworkId(), nic.getIPv4Address(), vmId, "UserVm");
    +                                    if (previousvmNetworkStats == null) {
    +                                        previousvmNetworkStats = new UserStatisticsVO(userVm.getAccountId(), userVm.getDataCenterId(),nic.getIPv4Address(), vmId, "UserVm", nic.getNetworkId());
    +                                        _userStatsDao.persist(previousvmNetworkStats);
    +                                    }
    +                                    UserStatisticsVO vmNetworkStat_lock = _userStatsDao.lock(userVm.getAccountId(), userVm.getDataCenterId(), nic.getNetworkId(), nic.getIPv4Address(), vmId, "UserVm");
    +
    +                                    if ((vmNetworkStat.getBytesSent() == 0) && (vmNetworkStat.getBytesReceived() == 0)) {
    +                                        s_logger.debug("bytes sent and received are all 0. Not updating user_statistics");
    --- End diff --
    
    Shouldn't we print the vmId here as well? Now you don't know which VM we are talking about


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