cayenne-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrus Adamchik <and...@objectstyle.org>
Subject Re: [cayenne] branch master updated: CAY-2589 Allow optionally using a local query cache that is separate from the shared query cache.
Date Thu, 04 Jul 2019 15:17:56 GMT

> I think that would be too limiting. I would prefer to be able to use caches with both
behavoirs/lifecycles in the same context.

Could you please elaborate on use cases?

> The choice of the scope or lifetime of the cache should be at the query level rather
than the context level.

But your current change does not do it at the query level. Both caches are the properties
of the context, not query. 

I am not (yet) worried about the unit tests. Those will become important once we figure out
a design. But I'd like this feature to be conceptually clear to the users.

When I am teaching Cayenne classes or just having an informal conversation about it, I always
feel that our cache "story" is too complicated. I can show various knobs, but I often have
hard time explaining when to use them and in what combination to solve a given problem. We
already have LOCAL vs SHARED cache. Having another type of LOCAL cache is going to cause someone's
head to explode :) So trying to figure out where the new feature fits in, and how to avoid
further complicating the cache model.

So maybe let's take a look at the use cases as I mentioned above to see why per-query switching
of the physical caches is important, and then brainstorm on how to best accomplish it.

> I'm out of the office until Monday, so I can't work on it until then, but
> we can keep discussing beforehand.

Appreciate that!

Andrus


> On Jul 4, 2019, at 5:50 PM, John Huss <johnthuss@gmail.com> wrote:
> 
> On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <andrus@objectstyle.org>
> wrote:
> 
>>> My intention was to separate the local and shared query caches while not
>>> eliminating either, so that is the reason both are used. So if you have
>>> explicitly set a custom localQueryCache then calling getQueryCache() will
>>> allow you to still access the *shared* query cache as well.
>> 
>> But this is called by the framework, not the user. And from what I see,
>> some of Cayenne code calls the former, and other - the later.
>> 
>> I general I am thinking we should make the type of per-context QueryCache
>> a property of the entire runtime (it is either a segment of the main cache,
>> or a separate per-context object ... either one or the other for all
>> contexts). Is this too limiting?
>> 
> 
> I think that would be too limiting. I would prefer to be able to use caches
> with both behavoirs/lifecycles in the same context. The choice of the scope
> or lifetime of the cache should be at the query level rather than the
> context level.
> 
> I realize now this commit didn't have unit tests to target the new
> behaviors when the localQueryCache is set. I have some tests for it in my
> application, but those didn't make it into Cayenne. And I need to write
> some more to check the behavior of the shared query cache when the
> localQueryCache is set.
> 
> I'm out of the office until Monday, so I can't work on it until then, but
> we can keep discussing beforehand. Thanks.
> 
>> On Jul 3, 2019, at 5:34 PM, John Huss <johnthuss@gmail.com> wrote:
>>> 
>>> On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <andrus@objectstyle.org>
>>> wrote:
>>> 
>>>> Hi John,
>>>> 
>>>> I think I understand where you are going with this feature. As we
>>>> discussed before, sometimes keeping a context cache as a transparent
>> region
>>>> of the main cache is undesirable, e.g. for memory management reasons.
>> But I
>>>> have some questions about the implementation committed to "master":
>>>> 
>>>> 1. The commit seems incomplete - sometimes we use the new
>>>> "getLocalQueryCache()", sometimes - the old "getQueryCache()".
>>>> 
>>> 
>>> My intention was to separate the local and shared query caches while not
>>> eliminating either, so that is the reason both are used. So if you have
>>> explicitly set a custom localQueryCache then calling getQueryCache() will
>>> allow you to still access the *shared* query cache as well.
>>> 
>>> 
>>> 
>>>> 2. Having both "queryCache" and "localQueryCache" as ivars of
>> BaseContext
>>>> may be confusing. What do you think of moving cache type selection
>> logic in
>>>> ObjectContextFactory?
>>>> 
>>> 
>>> Do you mean exposing this in the API of the interface? Currently I am
>>> configuring this with a DataContextFactory subclass like this:
>>> 
>>> *public* *static* *class* Factory *extends* DataContextFactory {
>>> 
>>> @Override
>>> 
>>> *protected* DataContext newInstance(DataChannel parent, ObjectStore
>>> objectStore) {
>>> 
>>> *return* *new* IcsDataContext(parent, objectStore);
>>> 
>>> }
>>> 
>>> @Override
>>> 
>>> *protected* ObjectContext createdFromDataDomain(DataDomain parent) {
>>> 
>>> DataContext result = (DataContext) *super*.createdFromDataDomain(parent);
>>> 
>>> result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*)); //
>> use
>>> separate unbounded cache whose lifetime is bound to the ObjectContext
>> itself
>>> 
>>> *return* result;
>>> 
>>> }
>>> 
>>> @Override
>>> 
>>> *protected* ObjectContext createFromDataContext(DataContext parent) {
>>> 
>>> DataContext result = (DataContext) *super*.createFromDataContext(parent);
>>> 
>>> result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*));
>>> 
>>> *return* result;
>>> 
>>> }
>>> 
>>> @Override
>>> 
>>> *protected* ObjectContext createFromGenericChannel(DataChannel parent) {
>>> 
>>> DataContext result = (DataContext)
>> *super*.createFromGenericChannel(parent);
>>> 
>>> result.setLocalQueryCache(*new* MapQueryCache(Integer.*MAX_VALUE*));
>>> 
>>> *return* result;
>>> 
>>> }
>>> 
>>> }
>>> 
>>> I'm open to other ways of configuring this if you have ideas, perhaps
>> like
>>> using DI. Thanks for your feedback.
>>> 
>>> 
>>> 
>>>> 
>>>> Thanks,
>>>> Andrus
>>>> 
>>>> 
>>>>> On Jul 1, 2019, at 5:53 PM, johnthuss@apache.org wrote:
>>>>> 
>>>>> This is an automated email from the ASF dual-hosted git repository.
>>>>> 
>>>>> johnthuss pushed a commit to branch master
>>>>> in repository https://gitbox.apache.org/repos/asf/cayenne.git
>>>>> 
>>>>> 
>>>>> The following commit(s) were added to refs/heads/master by this push:
>>>>>   new 597376a  CAY-2589 Allow optionally using a local query cache
>>>> that is separate from the shared query cache.
>>>>> 597376a is described below
>>>>> 
>>>>> commit 597376ae558b9a0c5ddc4390da188b9530204e3d
>>>>> Author: John Huss <johnthuss@apache.org>
>>>>> AuthorDate: Mon Jun 3 16:57:20 2019 -0500
>>>>> 
>>>>>  CAY-2589 Allow optionally using a local query cache that is separate
>>>> from the shared query cache.
>>>>> 
>>>>>  The local query cache can be customized by creating a custom
>>>> DataContextFactory.
>>>>> 
>>>>>  A separate cache can prevent memory leaks from occurring if you used
>>>> non-expiring cache
>>>>>  groups (like the default cache group perhaps) along with the local
>>>> cache, which wasn't
>>>>>  intuitive if you expected the lifetime of the cache to match the
>>>> lifetime of the ObjectContext.
>>>>> ---
>>>>> RELEASE-NOTES.txt                                  |  1 +
>>>>> .../main/java/org/apache/cayenne/BaseContext.java  | 27
>>>> +++++++++++++++++++++-
>>>>> .../cayenne/util/ObjectContextQueryAction.java     |  9 +++++++-
>>>>> 3 files changed, 35 insertions(+), 2 deletions(-)
>>>>> 
>>>>> diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
>>>>> index 6f45986..3aa2fa8 100644
>>>>> --- a/RELEASE-NOTES.txt
>>>>> +++ b/RELEASE-NOTES.txt
>>>>> @@ -36,6 +36,7 @@ CAY-2569 Custom 'Naming Strategy' in Cayenne Modeler
>>>>> CAY-2570 Use MySQL adapter for latest versions of MariaDB
>>>>> CAY-2579 Review and possibly relax usage of readonly flag of
>>>> ObjRelationship
>>>>> CAY-2585 Rename scalarQuery and params methods in SQLSelect
>>>>> +CAY-2589 - Allow optionally using a local query cache that is separate
>>>> from the shared query cache.
>>>>> 
>>>>> Bug Fixes:
>>>>> 
>>>>> diff --git
>>>> a/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
>>>> b/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
>>>>> index 981db73..ffae953 100644
>>>>> --- a/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
>>>>> +++ b/cayenne-server/src/main/java/org/apache/cayenne/BaseContext.java
>>>>> @@ -98,6 +98,7 @@ public abstract class BaseContext implements
>>>> ObjectContext {
>>>>>     // registry
>>>>>     protected transient DataChannel channel;
>>>>>     protected transient QueryCache queryCache;
>>>>> +     protected transient QueryCache localQueryCache;
>>>>>     protected transient EntityResolver entityResolver;
>>>>> 
>>>>>     protected boolean validatingObjectsOnCommit = true;
>>>>> @@ -469,17 +470,41 @@ public abstract class BaseContext implements
>>>> ObjectContext {
>>>>>     @Override
>>>>>     public abstract Collection<?> uncommittedObjects();
>>>>> 
>>>>> +     /**
>>>>> +      * Used for storing cached query results available to all
>>>> ObjectContexts.
>>>>> +      */
>>>>>     public QueryCache getQueryCache() {
>>>>>             attachToRuntimeIfNeeded();
>>>>>             return queryCache;
>>>>>     }
>>>>> 
>>>>>     /**
>>>>> -      * Sets a QueryCache to be used for storing cached query results.
>>>>> +      * Sets a QueryCache to be used for storing cached query results
>>>> available to all ObjectContexts.
>>>>>      */
>>>>>     public void setQueryCache(QueryCache queryCache) {
>>>>>             this.queryCache = queryCache;
>>>>>     }
>>>>> +
>>>>> +     /**
>>>>> +      * Used for storing cached query results available only to this
>>>> ObjectContext.
>>>>> +      * By default the local query cache and the shared query cache
>>>> will use the same underlying storage.
>>>>> +      *
>>>>> +      * @since 4.2
>>>>> +      */
>>>>> +     public QueryCache getLocalQueryCache() {
>>>>> +             attachToRuntimeIfNeeded();
>>>>> +             return localQueryCache != null ? localQueryCache :
>>>> getQueryCache();
>>>>> +     }
>>>>> +
>>>>> +     /**
>>>>> +      * Sets a QueryCache to be used for storing cached query results
>>>> available only to this ObjectContext.
>>>>> +      * By default the local query cache and the shared query cache
>>>> will use the same underlying storage.
>>>>> +      *
>>>>> +      * @since 4.2
>>>>> +      */
>>>>> +     public void setLocalQueryCache(QueryCache queryCache) {
>>>>> +             this.localQueryCache = queryCache;
>>>>> +     }
>>>>> 
>>>>>     /**
>>>>>      * Returns EventManager associated with the ObjectStore.
>>>>> diff --git
>>>> 
>> a/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
>>>> 
>> b/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
>>>>> index 75dbdfa..acef54e 100644
>>>>> ---
>>>> 
>> a/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
>>>>> +++
>>>> 
>> b/cayenne-server/src/main/java/org/apache/cayenne/util/ObjectContextQueryAction.java
>>>>> @@ -360,7 +360,7 @@ public abstract class ObjectContextQueryAction {
>>>>>           return !DONE;
>>>>>       }
>>>>> 
>>>>> -        QueryCache queryCache = getQueryCache();
>>>>> +        QueryCache queryCache = getLocalQueryCache();
>>>>>       QueryCacheEntryFactory factory = getCacheObjectFactory();
>>>>> 
>>>>>       if (cache) {
>>>>> @@ -385,6 +385,13 @@ public abstract class ObjectContextQueryAction {
>>>>>   protected QueryCache getQueryCache() {
>>>>>       return ((BaseContext) actingContext).getQueryCache();
>>>>>   }
>>>>> +
>>>>> +    /**
>>>>> +     * @since 4.2
>>>>> +     */
>>>>> +    protected QueryCache getLocalQueryCache() {
>>>>> +        return ((BaseContext) actingContext).getLocalQueryCache();
>>>>> +    }
>>>>> 
>>>>>   /**
>>>>>    * @since 3.0
>>>>> 
>>>> 
>>>> 
>> 
>> 


Mime
View raw message