mlsorensen commented on code in PR #7670:
URL: https://github.com/apache/cloudstack/pull/7670#discussion_r1244439545
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -1156,8 +1156,24 @@ public ResourceCountCheckTask() {
@Override
protected void runInContext() {
s_logger.info("Started resource counters recalculation periodic
task.");
- List<DomainVO> domains =
_domainDao.findImmediateChildrenForParent(Domain.ROOT_DOMAIN);
- List<AccountVO> accounts =
_accountDao.findActiveAccountsForDomain(Domain.ROOT_DOMAIN);
+ List<DomainVO> domains;
+ List<AccountVO> accounts;
+ // try/catch task, otherwise it won't be rescheduled in case of
exception
+ try {
+ domains =
_domainDao.findImmediateChildrenForParent(Domain.ROOT_DOMAIN);
+ } catch (Exception e) {
+ s_logger.warn("Resource counters recalculation periodic task
failed, unable to fetch immediate children for the domain " +
Domain.ROOT_DOMAIN, e);
+ // initialize domains as empty list to do best effort
recalculation
+ domains = new ArrayList<>();
+ }
+ // try/catch task, otherwise it won't be rescheduled in case of
exception
+ try {
+ accounts =
_accountDao.findActiveAccountsForDomain(Domain.ROOT_DOMAIN);
+ } catch (Exception e) {
+ s_logger.warn("Resource counters recalculation periodic task
failed, unable to fetch active accounts for domain " + Domain.ROOT_DOMAIN, e);
+ // initialize accounts as empty list to do best effort
recalculation
+ accounts = new ArrayList<>();
+ }
Review Comment:
> does it make sense to keep running if one of these retrievals fail (for
this pass I mean)?
Probably, I guess? At least we get the error logged now and if there is a
problem with that specific account or domain's data it won't affect processing
of the others.
> I like the logging but i think a little further restructuring makes sense;
two separate methods, one for domains and one for accounts for instance.
> the recursion at lines 1197 and 1204 looks quite strange, if at all it
works I would suggest a renaming for readability's sake.
I think changing the method names to be distinct has been addressed now, can
we resolve the conversation @DaanHoogland ?
--
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]
