cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jburwell <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Quota master
Date Fri, 21 Aug 2015 11:59:04 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message