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