winterhazel commented on code in PR #9888:
URL: https://github.com/apache/cloudstack/pull/9888#discussion_r1856614089
##########
engine/schema/src/main/java/com/cloud/usage/dao/UsageNetworksDaoImpl.java:
##########
@@ -70,11 +69,10 @@ public void remove(long networkId, Date removed) {
SearchCriteria<UsageNetworksVO> sc = this.createSearchCriteria();
sc.addAnd("networkId", SearchCriteria.Op.EQ, networkId);
sc.addAnd("removed", SearchCriteria.Op.NULL);
- UsageNetworksVO vo = findOneBy(sc);
- if (vo != null) {
- vo.setRemoved(removed);
- vo.setState(Network.State.Destroy.name());
- update(vo.getId(), vo);
+ List<UsageNetworksVO> usageNetworksVOs = listBy(sc);
+ for (UsageNetworksVO entry : usageNetworksVOs) {
+ entry.setRemoved(removed);
+ update(entry.getId(), entry);
Review Comment:
No. It does not make sense to update the helper entry's state.
Suppose Usage runs at 00:15 and aggregates daily, and that the user deletes
the network at 12:15.
If we update the entry's state to `Destroy`, on the next job execution,
Usage would generate a usage record for the network usage between 00:15 - 12:15
with the state set to `Destroy` (implying that the network's state was
`Destroy` between 00:15 - 12:15).
Then, Quota would inject `Destroy` as the network's state for that period
into activation rules. This could result in the incorrect value being billed
for tariffs based on the network's state.
--
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-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
