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 [email protected] or file a JIRA ticket
with INFRA.
---
