> 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 <johnth...@gmail.com> wrote:
>
> On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <and...@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 <johnth...@gmail.com> wrote:
>>>
>>> On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <and...@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, johnth...@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 <johnth...@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
>>>>>
>>>>
>>>>
>>
>>
