Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/689#discussion_r37627680 --- Diff: engine/schema/src/com/cloud/usage/dao/UsageDaoImpl.java --- @@ -469,4 +478,25 @@ public void removeOldUsageRecords(int days) { txn.close(); } } + + @SuppressWarnings("deprecation") + public Pair<List<? extends UsageVO>, Integer> getUsageRecordsPendingQuotaAggregation(final long accountId, final long domainId) { + final short opendb = TransactionLegacy.currentTxn().getDatabaseId(); + TransactionLegacy.open(TransactionLegacy.USAGE_DB).close(); + s_logger.debug("getting usage records for account: " + accountId + ", domainId: " + domainId); + Filter usageFilter = new Filter(UsageVO.class, "startDate", true, 0L, 10000L); + SearchCriteria<UsageVO> sc = createSearchCriteria(); + if (accountId != -1) { + sc.addAnd("accountId", SearchCriteria.Op.EQ, accountId); + } + if (domainId != -1) { + sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId); + } + sc.addAnd("quotaCalculated", SearchCriteria.Op.NULL); + sc.addOr("quotaCalculated", SearchCriteria.Op.EQ, 0); + s_logger.debug("Getting usage records" + usageFilter.getOrderBy()); + Pair<List<UsageVO>, Integer> usageRecords = searchAndCountAllRecords(sc, usageFilter); + TransactionLegacy.open(opendb).close(); --- End diff -- Reading through the ``TransactionLegacy`` code, this approach to switching the databases imposes in an incredibly high overhead because it actually consumes a connection from the pool and then throws it away in order to manipulate a thread local that tracks which the current connection pool. At a minimum, we should consider adding a ``switch`` or ``use`` method that only modifies the thread local tracking the current connection pool. The larger issue, which is out of scope for this PR, is that we essentially have a global, mutable state. This pattern of behavior appears to be present to change the connection pool used for the set of database queries executed within the scope of this method. If the method fails to switch the connection pool back to the original value, subsequent operations would fail. Within ``TransactionLegacy``, a thread local variable is used to track this state. Since threads are reused within application, this data is effectively global. We should refactor this class to create a new object instance for each transaction that points to the proper connection pool within its scope. By reducing the scope to an instance, we can create a side effect free transaction/connection management mechanism.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---