cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From marcaurele <>
Subject [GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...
Date Mon, 09 May 2016 06:10:50 GMT
Github user marcaurele commented on a diff in the pull request:
    --- Diff: framework/db/src/com/cloud/utils/db/ ---
    @@ -969,7 +969,12 @@ public T findByUuidIncludingRemoved(final String uuid) {
         public T findByIdIncludingRemoved(ID id) {
    -        return findById(id, true, null);
    +        if (_cache != null) {
    +            final Element element = _cache.get(id);
    +            return element == null ? findById(id, true, null) : (T)element.getObjectValue();
    +        } else {
    --- End diff --
    The test for the existence of the cache is made on other lines in this class, so I kept
it. My change is to use the cached value if there is one, as it wasn't the case before.
    The function looks pretty much the same as the `findById` (line 944).
    @DaanHoogland in your last code sample, on the first line you go first to the DB to retrieve
the object. Then if the cache isn't null, you fetch the value from the cache and discard the
one you just got. IMO it's a downgrade in terms of performance as you never use directly the
value in cache but always do a query against the DB.
    @jburwell I could change the code as you're suggesting, but then the exception should
be raised at other places too to keep the code consistent when the cache isn't configured,
which wasn't my initial goal.

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

View raw message