You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Andrus Adamchik <an...@objectstyle.org> on 2019/07/03 10:33:32 UTC
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
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()".
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?
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 <jo...@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
>
Process... Was : [cayenne] branch master updated: CAY-2589 Allow
optionally using a local query cache that is separate from the shared query
cache.
Posted by Andrus Adamchik <an...@objectstyle.org>.
Hi John,
Thanks for starting this meta-discussion about the process. I changed the email subject to separate it from the conversation about caching.
I think we all are doing a great job reviewing and voting on releases, but for better or for worse it is hard to set expectations for PR reviews and general discussion. While there is a lot of software downstream of Cayenne that heavily depends on it, still Apache Cayenne is a volunteer project. While everyone is doing their best, any one committer's attention may slip, or their interests / focus may be in some other part of the framework. So feedback can be uneven, as we've all seen many times.
That's just accepted as a part of the open source process, and Apache way of handling it is via "lazy consensus" (ask for feedback, but don't let the lack of it block your progress). As long as everyone is acting in good faith, it works the best for the long-term health of the project.
In this case there's nothing you could've done better. You gave more than enough time for anyone to chime in. It is not your fault that no one did. You totally had the right to commit the code when you did. In fact the fault is mine that I tuned out of PRs on GitHub, and only noticed that there is an ObjectContext API change when I pulled the repo to fix an unrelated issue.
As for the process... IMO the formal part is pretty straightforward (and we are already mostly following it) :
------
If a code change constitutes a feature or a bug fix (as opposed to say a javadoc change or code reformatting), do the following:
1. Open a Jira
2. When committing code, reference Jira ID in the commit message
3. Add a line in RELEASE-NOTES (and if you are breaking something - upgrade instructions in UPGRADE.txt)
This ties commits to features and features to releases, and clearly communicates changesets to the end users.
------
The rest of the process is more social and is harder to codify, so it will have to rely on good will and lazy consensus. IMO committers should use their judgement as to the amount of discussion and review they expect for a given feature. My only suggestion would be to "escalate" to dev@ from GitHub / Jira, if you want more people to pay attention.
I wonder if any other Apache projects were able to formalize the process better, considering the volunteer attention span constraint? I'd be open to suggestions.
> I would have appreciated some more credit
> since the test cases I submitted were often included wholesale into the
> correcting commits without any attribution. Personally I would have
> preferred a merge of the commit with my test case with a followup commit
> with the fix.
This should not happen. Merge is absolutely the way to handle it. We should all pay attention to such things.
Andrus
> On Jul 8, 2019, at 5:56 PM, John Huss <jo...@gmail.com> wrote:
>
>>
>> BTW, while we are having this discussion, may I request to move 597376ae5
>> commit to a pull request or a dedicated branch and off of master?
>>
>
> Certainly. This was on GitHub as a pull request
> <https://github.com/apache/cayenne/pull/388> for over two weeks before I
> committed it to master and I didn't receive any comments on it during that
> time, which was why I felt ok committing it to master. Should I expect pull
> requests, especially ones from committers, to sit without comment for that
> long?
>
> The first version <https://github.com/apache/cayenne/pull/292> of this
> change was also a pull request. It got some feedback initially, then after
> I made the requested changes the pull request sat open for basically an
> entire year without any more comments even after I requested them. In
> contrast, after I committed to master I got feedback in just a few days. If
> this is how interaction with the community (even a committer!) looks, then
> I'd suggest that we need to seriously look into ways of improving our
> process.
>
> I'd love to hear suggestions about how I can do this better in the future.
>
> On this topic, I don't think there is a process document or anything
> written up that indicates what steps need to be taken when committing code
> such as:
> 1) Creating a JIRA issue
> 2) Creating a pull request?
> 3) Emailing the dev list?
> 4) Updating the release notes
>
> On a more positive note, I was pleased with the responses I got to bugs I
> reported with recent changes that were made on master. Many of them were
> fixed very promptly! However, I would have appreciated some more credit
> since the test cases I submitted were often included wholesale into the
> correcting commits without any attribution. Personally I would have
> preferred a merge of the commit with my test case with a followup commit
> with the fix. But maybe I'm asking for too much.
>
> I do greatly value Cayenne and it plays a huge role in the software we
> make. I am very thankful for all the people who have contributed to it's
> success and continue to work to make it better.
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by John Huss <jo...@gmail.com>.
On Mon, Jul 8, 2019 at 2:57 AM Andrus Adamchik <an...@objectstyle.org>
wrote:
> My initial idea was that the LRU limits on the cache will serve as a
> transparent cleanup mechanism. I.e. the user would set the max objects per
> cache in a such a way, that even a completely full cache should not crash
> the app. E.g. here is an Ehcache config with a global LRU for all cache
> groups:
>
> <defaultCache maxEntriesLocalHeap="1000" eternal="false"
> memoryStoreEvictionPolicy="LRU">
> ...
> </defaultCache>
>
> It works well for use cases with very active, constantly churning the
> cache. In a less active app the churn rate won't be as high, and you'd be
> essentially overpaying for memory most of the time (though the app should
> still never run out of memory if it is configured with proper limits). So
> there seems to be two issues with the current implementation:
>
> * It requires developer awareness of cache sizing
> * The app will eventually be running at _max_ memory instead of _average_
>
> From your description, it looks like you don't really care about
> preserving both the current and the new implementation side by side. You
> just need a solution that works just like the old one, but is free of the
> limitations above?
>
Correct. It's important to be aware however, that the memory retained is
not purely the objects that are cached, but also every object fetched by
the ObjectContext the cached objects belonged to. Would it be possible to
null out the ObjectContext when object are inserted into the cache to
release this associated memory?
>
>
> Here is one quick idea on how we might handle that. We have a
> "QueryCache.clear()" method. This can become a way to explicitly clear the
> cache at the end of the context life. The method is currently deprecated,
> as we couldn't do a fast clean of only a subset of keys belonging to given
> context across all cache groups. But it doesn't have to be fast, and we can
> use Cayenne event threads to do it on the background without blocking the
> app. So maybe we undeprecate it (or create a new one, like
> "clearNonBlocking()" or something) and change the implementation to do a
> full scan of all cache keys, removing those matching the prefix for the
> ObjectContext (e.g. JCache has Cache.removeAll(Set<K> keys) method).
> WDYT?
>
Yes, I think that would work.
>
> BTW, while we are having this discussion, may I request to move 597376ae5
> commit to a pull request or a dedicated branch and off of master?
>
Certainly. This was on GitHub as a pull request
<https://github.com/apache/cayenne/pull/388> for over two weeks before I
committed it to master and I didn't receive any comments on it during that
time, which was why I felt ok committing it to master. Should I expect pull
requests, especially ones from committers, to sit without comment for that
long?
The first version <https://github.com/apache/cayenne/pull/292> of this
change was also a pull request. It got some feedback initially, then after
I made the requested changes the pull request sat open for basically an
entire year without any more comments even after I requested them. In
contrast, after I committed to master I got feedback in just a few days. If
this is how interaction with the community (even a committer!) looks, then
I'd suggest that we need to seriously look into ways of improving our
process.
I'd love to hear suggestions about how I can do this better in the future.
On this topic, I don't think there is a process document or anything
written up that indicates what steps need to be taken when committing code
such as:
1) Creating a JIRA issue
2) Creating a pull request?
3) Emailing the dev list?
4) Updating the release notes
On a more positive note, I was pleased with the responses I got to bugs I
reported with recent changes that were made on master. Many of them were
fixed very promptly! However, I would have appreciated some more credit
since the test cases I submitted were often included wholesale into the
correcting commits without any attribution. Personally I would have
preferred a merge of the commit with my test case with a followup commit
with the fix. But maybe I'm asking for too much.
I do greatly value Cayenne and it plays a huge role in the software we
make. I am very thankful for all the people who have contributed to it's
success and continue to work to make it better.
> > On Jul 5, 2019, at 6:45 PM, John Huss <jo...@gmail.com> wrote:
> >
> > Query results that are stored in the local cache are only
> visible/available
> > to the context that fetched them, so once the context has gone out of
> scope
> > these query results are no longer useful in any way - they simply can't
> be
> > accessed. Therefore it would be logical for those objects to be evicted
> > from the cache. But under the current behavior these objects will remain
> in
> > the cache until some expiration time configured for that cache group
> > expires. And crucially, not only the cached objects will remain, but the
> > entire context of the ObjectContext that fetched them will be retained in
> > memory. Configuring the expiration time for that cache group is
> cumbersome
> > and often the expiration time needs to be different for different
> > tasks/queries causing an explosion in the number of cache groups. Even
> when
> > an expiration has been set, during the period of time between when the
> > ObjectContext goes out of scope until the expiration fires this memory is
> > unavailable, which could be a problem.
> >
> > My first run at this problem added a finalizer to DataContext to clean up
> > the dead objects in the cache when the ObjectContext was garbage
> collected.
> > That worked ok, but I dislike finalizers, and even with this, under heavy
> > load there were leaks occurring that I couldn't resolve. After more
> thought
> > it made more sense to simply tie the lifetimes of the context and cache
> > together so that they will expire at the same time.
> >
> > Why do I want to use the local cache?
> > It's more simple to add a call to query.localCache() than to manually
> > create single-purpose custom caches all over the place, like:
> >
> > List<T> fetchResults;
> >
> > public List<T> fetchThings() {
> > if (resultResults != null) {
> > fetchResults = context.select(query);
> > }
> > return fetchResults;
> > }
> >
> > While this works and is easily understandable, using the built-in caching
> > behavior is much nicer. When the results of a fetch need to be accessed
> > multiple times using a cache can improve performance significantly.
> >
> > The current caching behavior can function as a hidden bomb. In one
> > situation a developer added a .localCache to a simple query that fetched
> a
> > single row essentially to define a lightweight un-modeled, read-only
> > relationship. But that query was called in multiple places, once of which
> > was a background process that fetches huge amounts of data. The query
> > wasn't using an explicit cache group, and the default cache group was not
> > configured with any expiration time, so before long the app ran out of
> > memory because it was retaining the results of these huge ObjectContext
> > forever. While you can blame the user for not configuring the cache
> group,
> > this still seems like less than ideal behavior.
> >
> > What do I want?
> > To be able to use the existing caching API .localCache and .sharedCache
> > without having to worry that my application will run out of memory if a
> > developer adds a .localCache call that doesn't get caught in code review.
> >
> > I agree that the current cache story is too complicated and I'd like to
> > help improve that if possible. I need some more time to think about my
> > ideas for that and how I would approach it.
> >
> > Thanks.
> >
> >
> > On Thu, Jul 4, 2019 at 10:18 AM Andrus Adamchik <an...@objectstyle.org>
> > wrote:
> >
> >>
> >>> 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 <jo...@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 <jo...@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 <jo...@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
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by Andrus Adamchik <an...@objectstyle.org>.
My initial idea was that the LRU limits on the cache will serve as a transparent cleanup mechanism. I.e. the user would set the max objects per cache in a such a way, that even a completely full cache should not crash the app. E.g. here is an Ehcache config with a global LRU for all cache groups:
<defaultCache maxEntriesLocalHeap="1000" eternal="false" memoryStoreEvictionPolicy="LRU">
...
</defaultCache>
It works well for use cases with very active, constantly churning the cache. In a less active app the churn rate won't be as high, and you'd be essentially overpaying for memory most of the time (though the app should still never run out of memory if it is configured with proper limits). So there seems to be two issues with the current implementation:
* It requires developer awareness of cache sizing
* The app will eventually be running at _max_ memory instead of _average_
From your description, it looks like you don't really care about preserving both the current and the new implementation side by side. You just need a solution that works just like the old one, but is free of the limitations above?
Here is one quick idea on how we might handle that. We have a "QueryCache.clear()" method. This can become a way to explicitly clear the cache at the end of the context life. The method is currently deprecated, as we couldn't do a fast clean of only a subset of keys belonging to given context across all cache groups. But it doesn't have to be fast, and we can use Cayenne event threads to do it on the background without blocking the app. So maybe we undeprecate it (or create a new one, like "clearNonBlocking()" or something) and change the implementation to do a full scan of all cache keys, removing those matching the prefix for the ObjectContext (e.g. JCache has Cache.removeAll(Set<K> keys) method).
WDYT?
BTW, while we are having this discussion, may I request to move 597376ae5 commit to a pull request or a dedicated branch and off of master?
Andrus
> On Jul 5, 2019, at 6:45 PM, John Huss <jo...@gmail.com> wrote:
>
> Query results that are stored in the local cache are only visible/available
> to the context that fetched them, so once the context has gone out of scope
> these query results are no longer useful in any way - they simply can't be
> accessed. Therefore it would be logical for those objects to be evicted
> from the cache. But under the current behavior these objects will remain in
> the cache until some expiration time configured for that cache group
> expires. And crucially, not only the cached objects will remain, but the
> entire context of the ObjectContext that fetched them will be retained in
> memory. Configuring the expiration time for that cache group is cumbersome
> and often the expiration time needs to be different for different
> tasks/queries causing an explosion in the number of cache groups. Even when
> an expiration has been set, during the period of time between when the
> ObjectContext goes out of scope until the expiration fires this memory is
> unavailable, which could be a problem.
>
> My first run at this problem added a finalizer to DataContext to clean up
> the dead objects in the cache when the ObjectContext was garbage collected.
> That worked ok, but I dislike finalizers, and even with this, under heavy
> load there were leaks occurring that I couldn't resolve. After more thought
> it made more sense to simply tie the lifetimes of the context and cache
> together so that they will expire at the same time.
>
> Why do I want to use the local cache?
> It's more simple to add a call to query.localCache() than to manually
> create single-purpose custom caches all over the place, like:
>
> List<T> fetchResults;
>
> public List<T> fetchThings() {
> if (resultResults != null) {
> fetchResults = context.select(query);
> }
> return fetchResults;
> }
>
> While this works and is easily understandable, using the built-in caching
> behavior is much nicer. When the results of a fetch need to be accessed
> multiple times using a cache can improve performance significantly.
>
> The current caching behavior can function as a hidden bomb. In one
> situation a developer added a .localCache to a simple query that fetched a
> single row essentially to define a lightweight un-modeled, read-only
> relationship. But that query was called in multiple places, once of which
> was a background process that fetches huge amounts of data. The query
> wasn't using an explicit cache group, and the default cache group was not
> configured with any expiration time, so before long the app ran out of
> memory because it was retaining the results of these huge ObjectContext
> forever. While you can blame the user for not configuring the cache group,
> this still seems like less than ideal behavior.
>
> What do I want?
> To be able to use the existing caching API .localCache and .sharedCache
> without having to worry that my application will run out of memory if a
> developer adds a .localCache call that doesn't get caught in code review.
>
> I agree that the current cache story is too complicated and I'd like to
> help improve that if possible. I need some more time to think about my
> ideas for that and how I would approach it.
>
> Thanks.
>
>
> On Thu, Jul 4, 2019 at 10:18 AM Andrus Adamchik <an...@objectstyle.org>
> wrote:
>
>>
>>> 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 <jo...@gmail.com> wrote:
>>>
>>> On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <an...@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 <jo...@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 <jo...@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
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by John Huss <jo...@gmail.com>.
Query results that are stored in the local cache are only visible/available
to the context that fetched them, so once the context has gone out of scope
these query results are no longer useful in any way - they simply can't be
accessed. Therefore it would be logical for those objects to be evicted
from the cache. But under the current behavior these objects will remain in
the cache until some expiration time configured for that cache group
expires. And crucially, not only the cached objects will remain, but the
entire context of the ObjectContext that fetched them will be retained in
memory. Configuring the expiration time for that cache group is cumbersome
and often the expiration time needs to be different for different
tasks/queries causing an explosion in the number of cache groups. Even when
an expiration has been set, during the period of time between when the
ObjectContext goes out of scope until the expiration fires this memory is
unavailable, which could be a problem.
My first run at this problem added a finalizer to DataContext to clean up
the dead objects in the cache when the ObjectContext was garbage collected.
That worked ok, but I dislike finalizers, and even with this, under heavy
load there were leaks occurring that I couldn't resolve. After more thought
it made more sense to simply tie the lifetimes of the context and cache
together so that they will expire at the same time.
Why do I want to use the local cache?
It's more simple to add a call to query.localCache() than to manually
create single-purpose custom caches all over the place, like:
List<T> fetchResults;
public List<T> fetchThings() {
if (resultResults != null) {
fetchResults = context.select(query);
}
return fetchResults;
}
While this works and is easily understandable, using the built-in caching
behavior is much nicer. When the results of a fetch need to be accessed
multiple times using a cache can improve performance significantly.
The current caching behavior can function as a hidden bomb. In one
situation a developer added a .localCache to a simple query that fetched a
single row essentially to define a lightweight un-modeled, read-only
relationship. But that query was called in multiple places, once of which
was a background process that fetches huge amounts of data. The query
wasn't using an explicit cache group, and the default cache group was not
configured with any expiration time, so before long the app ran out of
memory because it was retaining the results of these huge ObjectContext
forever. While you can blame the user for not configuring the cache group,
this still seems like less than ideal behavior.
What do I want?
To be able to use the existing caching API .localCache and .sharedCache
without having to worry that my application will run out of memory if a
developer adds a .localCache call that doesn't get caught in code review.
I agree that the current cache story is too complicated and I'd like to
help improve that if possible. I need some more time to think about my
ideas for that and how I would approach it.
Thanks.
On Thu, Jul 4, 2019 at 10:18 AM Andrus Adamchik <an...@objectstyle.org>
wrote:
>
> > 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 <jo...@gmail.com> wrote:
> >
> > On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <an...@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 <jo...@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 <jo...@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
> >>>>>
> >>>>
> >>>>
> >>
> >>
>
>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by Andrus Adamchik <an...@objectstyle.org>.
> 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 <jo...@gmail.com> wrote:
>
> On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <an...@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 <jo...@gmail.com> wrote:
>>>
>>> On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <an...@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 <jo...@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
>>>>>
>>>>
>>>>
>>
>>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by John Huss <jo...@gmail.com>.
On Thu, Jul 4, 2019 at 2:45 AM Andrus Adamchik <an...@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 <jo...@gmail.com> wrote:
> >
> > On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <an...@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 <jo...@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
> >>>
> >>
> >>
>
>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by Andrus Adamchik <an...@objectstyle.org>.
> 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?
Andrus
> On Jul 3, 2019, at 5:34 PM, John Huss <jo...@gmail.com> wrote:
>
> On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <an...@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 <jo...@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
>>>
>>
>>
Re: [cayenne] branch master updated: CAY-2589 Allow optionally using
a local query cache that is separate from the shared query cache.
Posted by John Huss <jo...@gmail.com>.
On Wed, Jul 3, 2019 at 5:33 AM Andrus Adamchik <an...@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 <jo...@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
> >
>
>