[GitHub] rafaelweingartner commented on a change in pull request #3078: Add influxdb to statscollector

[GitBox <gi...@apache.org>]

rafaelweingartner commented on a change in pull request #3078: Add influxdb to statscollector
URL: https://github.com/apache/cloudstack/pull/3078#discussion_r239605451
 
 

 ##########
 File path: server/src/main/java/com/cloud/server/StatsCollector.java
 ##########
 @@ -420,55 +505,65 @@ protected void runInContext() {
                 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.NEQ, Host.Type.Storage.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.ConsoleProxy.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.SecondaryStorage.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.LocalSecondaryStorage.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.TrafficMonitor.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.SecondaryStorageVM.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.ExternalFirewall.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.ExternalLoadBalancer.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.NetScalerControlCenter.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.L2Networking.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.BaremetalDhcp.toString());
-                sc.addAnd("type", SearchCriteria.Op.NEQ, Host.Type.BaremetalPxe.toString());
+                sc.addAnd("type", SearchCriteria.Op.EQ, Host.Type.Routing.toString());
+
                 ConcurrentHashMap<Long, HostStats> hostStats = new ConcurrentHashMap<Long, HostStats>();
+                Map<Object, Object> metrics = new HashMap<>();
                 List<HostVO> hosts = _hostDao.search(sc, null);
+
                 for (HostVO host : hosts) {
-                    HostStatsEntry stats = (HostStatsEntry)_resourceMgr.getHostStatistics(host.getId());
-                    if (stats != null) {
-                        hostStats.put(host.getId(), stats);
+                    HostStatsEntry hostStatsEntry = (HostStatsEntry)_resourceMgr.getHostStatistics(host.getId());
+                    if (hostStatsEntry != null) {
+                        hostStatsEntry.setHostVo(host);
+                        metrics.put(hostStatsEntry.getHostId(), hostStatsEntry);
+                        _hostStats.put(host.getId(), hostStatsEntry);
                     } else {
                         s_logger.warn("Received invalid host stats for host: " + host.getId());
                     }
                 }
-                _hostStats = hostStats;
-                // Get a subset of hosts with GPU support from the list of "hosts"
-                List<HostVO> gpuEnabledHosts = new ArrayList<HostVO>();
-                if (hostIds != null) {
-                    for (HostVO host : hosts) {
-                        if (hostIds.contains(host.getId())) {
-                            gpuEnabledHosts.add(host);
-                        }
-                    }
-                } else {
-                    // Check for all the hosts managed by CloudStack.
-                    gpuEnabledHosts = hosts;
-                }
-                for (HostVO host : gpuEnabledHosts) {
-                    HashMap<String, HashMap<String, VgpuTypesInfo>> groupDetails = _resourceMgr.getGPUStatistics(host);
-                    if (groupDetails != null) {
-                        _resourceMgr.updateGPUDetails(host.getId(), groupDetails);
-                    }
+
+                if (externalStatsType == ExternalStatsProtocol.INFLUXDB) {
+                    sendMetricsToInfluxdb(metrics);
                 }
-                hostIds = _hostGpuGroupsDao.listHostIds();
+
+                updateGpuEnabledHostsDetails(hosts);
             } catch (Throwable t) {
                 s_logger.error("Error trying to retrieve host stats", t);
             }
         }
+
+        /**
+         * Updates GPU details on hosts supporting GPU.
+         */
+        private void updateGpuEnabledHostsDetails(List<HostVO> hosts) {
 
 Review comment:
   This method is very odd. Why not load all host ids that have GPUs as the first thing? And then proceed with the execution? It is weird to execute once, then load some state data, and then in the second execution, you use that data (in this context, it is not making much sense).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services