olivierlemasle commented on a change in pull request #5017:
URL: https://github.com/apache/cloudstack/pull/5017#discussion_r633770306
##########
File path:
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
##########
@@ -653,6 +653,16 @@ public boolean restoreBackupVolumeAndAttachToVM(final
String backedUpVolumeUuid,
return true;
}
+ private void unassignBackupOffering(VMInstanceVO vm) {
Review comment:
This method is now unused, right? Please remove it.
##########
File path:
engine/schema/src/main/java/com/cloud/event/dao/UsageEventDaoImpl.java
##########
@@ -75,6 +77,13 @@ public UsageEventDaoImpl() {
IpeventsSearch.or("releaseEvent", IpeventsSearch.entity().getType(),
SearchCriteria.Op.EQ);
IpeventsSearch.cp();
IpeventsSearch.done();
+
+ EventsSearch = createSearchBuilder();
Review comment:
Standard convention for variables is to use camelCase, starting with a
lowercase.
##########
File path: usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java
##########
@@ -83,6 +91,12 @@ public static boolean parse(AccountVO account, Date
startDate, Date endDate) {
for (final BackupInfo backupInfo : vmUsageMap.values()) {
final Long vmId = backupInfo.getVmId();
+
+ List<UsageEventVO> backupUsageEvents =
s_usageEventDao.listEventsInTimeRange(EventTypes.EVENT_VM_BACKUP_USAGE_METRIC,
vmId, startDate, endDate);
+ if (CollectionUtils.isEmpty(backupUsageEvents)) {
+ continue;
Review comment:
This assumes `backup.framework.sync.interval` <
`usage.stats.job.aggregation.range`, right? (i.e. backup metrics are updated
more frequently that usage jobs)
If `backup.framework.sync.interval` >= `usage.stats.job.aggregation.range`,
I suppose we will miss usage records.
--
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:
[email protected]
