Damans227 commented on code in PR #14310:
URL: https://github.com/apache/cloudstack/pull/14310#discussion_r4219296407
##########
engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java:
##########
@@ -42,19 +43,50 @@ public class UsageBackupDaoImpl extends
GenericDaoBase<UsageBackupVO, Long> impl
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final Long vmId, Long backupOfferingId, final
Long size, final Long virtualSize) {
- try (TransactionLegacy txn =
TransactionLegacy.open(TransactionLegacy.USAGE_DB)) {
- SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
- sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
- sc.addAnd("backupOfferingId", SearchCriteria.Op.EQ,
backupOfferingId);
- UsageBackupVO vo = findOneBy(sc);
- if (vo != null) {
- vo.setSize(size);
- vo.setProtectedSize(virtualSize);
- update(vo.getId(), vo);
+ public List<UsageBackupVO> listActiveUsage(Long vmId, Long
backupOfferingId) {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ sc.addAnd("backupOfferingId", SearchCriteria.Op.EQ, backupOfferingId);
+ sc.addAnd("removed", SearchCriteria.Op.NULL);
+ return listBy(sc, new Filter(UsageBackupVO.class, "created", false));
+ }
+
+ @Override
+ public void updateMetrics(final Long vmId, final Long backupOfferingId,
final Long size, final Long virtualSize, final Date eventDate) {
+ final long newSize = size != null ? size : 0L;
+ final long newProtectedSize = virtualSize != null ? virtualSize : 0L;
+ TransactionLegacy txn =
TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ txn.start();
+ List<UsageBackupVO> activeUsage = listActiveUsage(vmId,
backupOfferingId);
+ if (activeUsage.isEmpty()) {
+ logger.warn("No active backup usage for VM [{}] and backup
offering [{}], ignoring backup metrics of size [{}] and protected size [{}].",
+ vmId, backupOfferingId, newSize, newProtectedSize);
+ txn.commit();
+ return;
+ }
+
+ UsageBackupVO latest = activeUsage.get(0);
+ if (activeUsage.size() == 1 && latest.getSize() == newSize &&
latest.getProtectedSize() == newProtectedSize) {
+ txn.commit();
+ return;
+ }
+
+ // Close the active rows and open one with the new size; this also
merges duplicates.
+ for (UsageBackupVO usage : activeUsage) {
+ usage.setRemoved(eventDate);
+ update(usage.getId(), usage);
}
+ UsageBackupVO newUsage = new UsageBackupVO(latest.getZoneId(),
latest.getAccountId(), latest.getDomainId(), vmId, backupOfferingId, eventDate);
+ newUsage.setSize(newSize);
+ newUsage.setProtectedSize(newProtectedSize);
+ persist(newUsage);
+ txn.commit();
} catch (final Exception e) {
+ txn.rollback();
logger.error("Error updating backup metrics: " + e.getMessage(),
e);
Review Comment:
does it reach the usage job though? this catch only logs it and doesnt throw
again, so the event still gets marked as done
##########
engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql:
##########
@@ -18,3 +18,15 @@
--;
-- Schema upgrade from 4.22.1.0 to 4.22.2.0
--;
+
+-- Last backup usage metric published per VM and backup offering
+CREATE TABLE IF NOT EXISTS `cloud`.`backup_usage_metric` (
Review Comment:
makes sense, waiting on #13933 works for me
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]