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 2022/03/07 13:15:38 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5984: Persistence of VM stats
GutoVeronezi commented on a change in pull request #5984:
URL: https://github.com/apache/cloudstack/pull/5984#discussion_r820678684
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
##########
@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm.dao;
+
+import java.util.Date;
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.VmStatsVO;
+
+/**
+ * Data Access Object for vm_stats table.
+ */
+public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
+
+ /**
+ * Find VM stats by VM ID.
+ * @param vmId the VM ID.
+ * @return list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmId(long vmId);
+
+ /**
+ * Find VM stats by VM ID. The result is sorted by timestamp in descending order.
+ * @param vmId the VM ID.
+ * @return ordered list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmIdOrderByTimestampDesc(long vmId);
+
+ /**
+ * Find stats by VM ID and timestamp >= a given time.
+ * @param vmId the specific VM.
+ * @param time the specific time.
+ * @return list of stats for the specified VM, with timestamp >= the specified time.
+ */
+ List<VmStatsVO> findByVmIdAndTimestampGreaterThanEqual(long vmId, Date time);
+
+ /**
+ * Find stats by VM ID and timestamp <= a given time.
Review comment:
```suggestion
* Finds stats by VM ID and timestamp <= a given time.
```
##########
File path: plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java
##########
@@ -134,6 +155,166 @@ private void updateHostMetrics(final Metrics metrics, final HostJoinVO host) {
metrics.setMaximumMemoryUsage((long) hostStats.getUsedMemory());
}
}
+ /**
+ * Searches for VM stats based on the {@code ListVMsUsageHistoryCmd} parameters.
+ *
+ * @param cmd the {@link ListVMsUsageHistoryCmd} specifying what should be searched.
+ * @return the list of VM metrics stats found.
+ */
+ @Override
+ public ListResponse<VmMetricsStatsResponse> searchForVmMetricsStats(ListVMsUsageHistoryCmd cmd) {
+ Pair<List<UserVmVO>, Integer> userVmList = searchForUserVmsInternal(cmd);
+ Map<Long,List<VmStatsVO>> vmStatsList = searchForVmMetricsStatsInternal(cmd, userVmList.first());
+ return createVmMetricsStatsResponse(userVmList, vmStatsList);
+ }
+
+ /**
+ * Searches VMs based on {@code ListVMsUsageHistoryCmd} parameters.
+ *
+ * @param cmd the {@link ListVMsUsageHistoryCmd} specifying the parameters.
+ * @return the list of VMs.
+ */
+ protected Pair<List<UserVmVO>, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) {
+ Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
+ List<Long> ids = getIdsListFromCmd(cmd.getId(), cmd.getIds());
+ String name = cmd.getName();
+ String keyword = cmd.getKeyword();
+
+ SearchBuilder<UserVmVO> sb = userVmDao.createSearchBuilder();
+ sb.and("idIN", sb.entity().getId(), SearchCriteria.Op.IN);
+ sb.and("displayName", sb.entity().getDisplayName(), SearchCriteria.Op.LIKE);
+ sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
+
+ SearchCriteria<UserVmVO> sc = sb.create();
+ if (CollectionUtils.isNotEmpty(ids)) {
+ sc.setParameters("idIN", ids.toArray());
+ }
+ if (StringUtils.isNotBlank(name)) {
+ sc.setParameters("displayName", "%" + name + "%");
+ }
+ if (StringUtils.isNotBlank(keyword)) {
+ SearchCriteria<UserVmVO> ssc = userVmDao.createSearchCriteria();
+ ssc.addOr("displayName", SearchCriteria.Op.LIKE, "%" + keyword + "%");
+ ssc.addOr("state", SearchCriteria.Op.EQ, keyword);
+ sc.addAnd("displayName", SearchCriteria.Op.SC, ssc);
+ }
+
+ return userVmDao.searchAndCount(sc, searchFilter);
+ }
+
+ /**
+ * Searches stats for a list of VMs, based on date filtering parameters.
+ *
+ * @param cmd the {@link ListVMsUsageHistoryCmd} specifying the filtering parameters.
+ * @param userVmList the list of VMs for which stats should be searched.
+ * @return the key-value map in which keys are VM IDs and values are lists of VM stats.
+ */
+ protected Map<Long,List<VmStatsVO>> searchForVmMetricsStatsInternal(ListVMsUsageHistoryCmd cmd, List<UserVmVO> userVmList) {
+ Map<Long,List<VmStatsVO>> vmStatsVOList = new HashMap<Long,List<VmStatsVO>>();
+ Date startDate = cmd.getStartDate();
+ Date endDate = cmd.getEndDate();
+
+ validateDateParams(startDate, endDate);
+
+ for (UserVmVO userVmVO : userVmList) {
+ Long vmId = userVmVO.getId();
+ vmStatsVOList.put(vmId, findVmStatsAccordingToDateParams(vmId, startDate, endDate));
+ }
+
+ return vmStatsVOList;
+ }
+
+ /**
+ * Checks if {@code startDate} is after {@code endDate} (when both are not null)
+ * and throws an {@link InvalidParameterValueException} if so.
+ *
+ * @param startDate the start date to be validated.
+ * @param endDate the end date to be validated.
+ */
+ protected void validateDateParams(Date startDate, Date endDate) {
+ if ((startDate != null && endDate != null) && (startDate.after(endDate))){
+ throw new InvalidParameterValueException("startDate cannot be after endDate.");
+ }
+ }
+
+ /**
+ * Finds stats for a specific VM based on date parameters.
+ *
+ * @param vmId the specific VM.
+ * @param startDate the start date to filtering.
+ * @param endDate the end date to filtering.
+ * @return the list of stats for the specified VM.
+ */
+ protected List<VmStatsVO> findVmStatsAccordingToDateParams(Long vmId, Date startDate, Date endDate){
+ if (startDate != null && endDate != null) {
+ return vmStatsDao.findByVmIdAndTimestampBetween(vmId, startDate, endDate);
+ } else if (startDate != null) {
+ return vmStatsDao.findByVmIdAndTimestampGreaterThanEqual(vmId, startDate);
+ } else if (endDate != null) {
+ return vmStatsDao.findByVmIdAndTimestampLessThanEqual(vmId, endDate);
+ }
Review comment:
If the block returns something, then we don't need the else statement. We could remove them:
```suggestion
if (startDate != null && endDate != null) {
return vmStatsDao.findByVmIdAndTimestampBetween(vmId, startDate, endDate);
}
if (startDate != null) {
return vmStatsDao.findByVmIdAndTimestampGreaterThanEqual(vmId, startDate);
}
if (endDate != null) {
return vmStatsDao.findByVmIdAndTimestampLessThanEqual(vmId, endDate);
}
```
##########
File path: server/src/main/java/com/cloud/server/StatsCollector.java
##########
@@ -632,8 +649,90 @@ protected Point creteInfluxDbPoint(Object metricsObject) {
}
}
- public VmStats getVmStats(long id) {
- return _VmStats.get(id);
+ /**
+ * <p>Previously, the VM stats cleanup process was triggered during the data collection process.
+ * So, when data collection was disabled, the cleaning process was also disabled.</p>
+ *
+ * <p>With the introduction of persistence of VM stats, as well as the provision of historical data,
+ * we created this class to allow that both the collection process and the data cleaning process
+ * can be enabled/disabled independently.</p>
+ */
+ class VmStatsCleaner extends ManagedContextRunnable{
+ protected void runInContext() {
+ cleanUpVirtualMachineStats();
+ }
+ }
+
+ /**
+ * Gets the latest or the accumulation of the stats collected from a given VM.
+ *
+ * @param vmId the specific VM.
+ * @param accumulate whether or not the stats data should be accumulated.
+ * @return the latest or the accumulation of the stats for the specified VM.
+ */
+ public VmStats getVmStats(long vmId, Boolean accumulate) {
+ List<VmStatsVO> vmStatsVOList = vmStatsDao.findByVmIdOrderByTimestampDesc(vmId);
+
+ if (CollectionUtils.isNotEmpty(vmStatsVOList)) {
Review comment:
We could invert this statement to reduce indentation of the next block.
##########
File path: api/src/main/java/org/apache/cloudstack/api/response/StatsResponse.java
##########
@@ -0,0 +1,147 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import java.util.Date;
+
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseResponse;
+
+import com.cloud.serializer.Param;
+import com.google.gson.annotations.SerializedName;
+
+public class StatsResponse extends BaseResponse {
+
+ @SerializedName("timestamp")
+ @Param(description = "the time when the VM stats were collected. The format is \"yyyy-MM-dd hh:mm:ss\"")
+ private Date timestamp;
+
+ @SerializedName("cpuused")
+ @Param(description = "the amount (percentage) of the VM's CPU currently used")
+ private String cpuUsed;
+
+ @SerializedName(ApiConstants.DISK_IO_READ)
+ @Param(description = "the VM's disk read (IO)")
+ protected Long diskIORead;
+
+ @SerializedName(ApiConstants.DISK_IO_WRITE)
+ @Param(description = "the VM's disk write (IO)")
+ protected Long diskIOWrite;
+
+ @SerializedName(ApiConstants.DISK_IO_PSTOTAL)
+ @Param(description = "the total disk iops")
+ protected Long diskIopsTotal = 0L;
+
+ @SerializedName(ApiConstants.DISK_KBS_READ)
+ @Param(description = "the VM's disk read (bytes)")
+ private Long diskKbsRead;
+
+ @SerializedName(ApiConstants.DISK_KBS_WRITE)
+ @Param(description = "the VM's disk write (bytes)")
+ private Long diskKbsWrite;
+
+ @SerializedName("memoryintfreekbs")
+ @Param(description = "the internal memory free of the VM or zero if it cannot be calculated")
+ private Long memoryIntFreeKBs;
+
+ @SerializedName("memorykbs")
+ @Param(description = "the memory used by the VM in Kbps")
+ private Long memoryKBs;
+
+ @SerializedName("memorytargetkbs")
+ @Param(description = "the target memory in VM in Kbps")
+ private Long memoryTargetKBs;
+
+ @SerializedName("networkkbsread")
+ @Param(description = "the incoming network traffic on the VM")
+ protected Long networkKbsRead;
+
+ @SerializedName("networkkbswrite")
+ @Param(description = "the outgoing network traffic on the host")
+ protected Long networkKbsWrite;
+
+ @SerializedName("networkread")
+ @Param(description = "the network read in MiB")
+ protected String networkRead;
+
+ @SerializedName("networkwrite")
+ @Param(description = "the network write in MiB")
+ protected String networkWrite;
+
+ public void setTimestamp(Date timestamp) {
+ this.timestamp = timestamp;
+ }
+
+ public void setCpuUsed(String cpuUsed) {
+ this.cpuUsed = cpuUsed;
+ }
+
+ public void setDiskIORead(Long diskIORead) {
+ this.diskIORead = diskIORead;
+ accumulateDiskIopsTotal(diskIORead);
+ }
+
+ public void setDiskIOWrite(Long diskIOWrite) {
+ this.diskIOWrite = diskIOWrite;
+ accumulateDiskIopsTotal(diskIOWrite);
+ }
+
+ public void setDiskKbsRead(Long diskKbsRead) {
+ this.diskKbsRead = diskKbsRead;
+ }
+
+ public void setDiskKbsWrite(Long diskKbsWrite) {
+ this.diskKbsWrite = diskKbsWrite;
+ }
+
+ public void setMemoryIntFreeKBs(Long memoryIntFreeKBs) {
+ this.memoryIntFreeKBs = memoryIntFreeKBs;
+ }
+
+ public void setMemoryKBs(Long memoryKBs) {
+ this.memoryKBs = memoryKBs;
+ }
+
+ public void setMemoryTargetKBs(Long memoryTargetKBs) {
+ this.memoryTargetKBs = memoryTargetKBs;
+ }
+
+ public void setNetworkKbsRead(Long networkKbsRead) {
+ this.networkKbsRead = networkKbsRead;
+ if (networkKbsRead != null) {
+ this.networkRead = String.format("%.2f MB", networkKbsRead / 1024.0);
+ }
+ }
+
+ public void setNetworkKbsWrite(Long networkKbsWrite) {
+ this.networkKbsWrite = networkKbsWrite;
+ if (networkKbsWrite != null) {
+ this.networkWrite = String.format("%.2f MB", networkKbsWrite / 1024.0);
+ }
+ }
+
+ /**
+ * Accumulate disk IOPS (Input/Output Operations Per Second)
Review comment:
```suggestion
* Accumulates disk IOPS (Input/Output Operations Per Second)
```
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
##########
@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm.dao;
+
+import java.util.Date;
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.VmStatsVO;
+
+/**
+ * Data Access Object for vm_stats table.
+ */
+public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
+
+ /**
+ * Find VM stats by VM ID.
Review comment:
```suggestion
* Finds VM stats by VM ID.
```
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
##########
@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm.dao;
+
+import java.util.Date;
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.VmStatsVO;
+
+/**
+ * Data Access Object for vm_stats table.
+ */
+public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
+
+ /**
+ * Find VM stats by VM ID.
+ * @param vmId the VM ID.
+ * @return list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmId(long vmId);
+
+ /**
+ * Find VM stats by VM ID. The result is sorted by timestamp in descending order.
+ * @param vmId the VM ID.
+ * @return ordered list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmIdOrderByTimestampDesc(long vmId);
+
+ /**
+ * Find stats by VM ID and timestamp >= a given time.
Review comment:
```suggestion
* Finds stats by VM ID and timestamp >= a given time.
```
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
##########
@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm.dao;
+
+import java.util.Date;
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.VmStatsVO;
+
+/**
+ * Data Access Object for vm_stats table.
+ */
+public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
+
+ /**
+ * Find VM stats by VM ID.
+ * @param vmId the VM ID.
+ * @return list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmId(long vmId);
+
+ /**
+ * Find VM stats by VM ID. The result is sorted by timestamp in descending order.
+ * @param vmId the VM ID.
+ * @return ordered list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmIdOrderByTimestampDesc(long vmId);
+
+ /**
+ * Find stats by VM ID and timestamp >= a given time.
+ * @param vmId the specific VM.
+ * @param time the specific time.
+ * @return list of stats for the specified VM, with timestamp >= the specified time.
+ */
+ List<VmStatsVO> findByVmIdAndTimestampGreaterThanEqual(long vmId, Date time);
+
+ /**
+ * Find stats by VM ID and timestamp <= a given time.
+ * @param vmId the specific VM.
+ * @param time the specific time.
+ * @return list of stats for the specified VM, with timestamp <= the specified time.
+ */
+ List<VmStatsVO> findByVmIdAndTimestampLessThanEqual(long vmId, Date time);
+
+ /**
+ * Find stats by VM ID and timestamp between a given time range.
Review comment:
```suggestion
* Finds stats by VM ID and timestamp between a given time range.
```
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java
##########
@@ -0,0 +1,82 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.vm.dao;
+
+import java.util.Date;
+import java.util.List;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.VmStatsVO;
+
+/**
+ * Data Access Object for vm_stats table.
+ */
+public interface VmStatsDao extends GenericDao<VmStatsVO, Long> {
+
+ /**
+ * Find VM stats by VM ID.
+ * @param vmId the VM ID.
+ * @return list of stats for the specified VM.
+ */
+ List<VmStatsVO> findByVmId(long vmId);
+
+ /**
+ * Find VM stats by VM ID. The result is sorted by timestamp in descending order.
Review comment:
```suggestion
* Finds VM stats by VM ID. The result is sorted by timestamp in descending order.
```
##########
File path: api/src/main/java/org/apache/cloudstack/api/ApiArgValidator.java
##########
@@ -18,6 +18,19 @@
package org.apache.cloudstack.api;
public enum ApiArgValidator {
- NotNullOrEmpty, // does StringUtils.isEmpty check
+ /**
PositiveNumber, // does != null and > 0 check
Review comment:
We can remove this line.
--
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