Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reword and clarify the 2LC docs #785

Merged
merged 1 commit into from
Feb 7, 2019
Merged

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Feb 6, 2019

I've massaged the wording a bit further to clarify the implications of non-clustered caching.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a couple of comments inline.

Entity collections need to be individually annotated to be cached:
When an entity is annotated with `@Cache`, all its field values are cached except for collections and relations to other entities.

This means the entity can be loaded without querying the database, but be careful as it implies the loaded entity might not reflect recent changes in the database.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really true? I mean if you only have a local cache, it should always be up to date, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd be right on "traditional Hibernate usage". But not in Protean.. That's precisely why I'm rewording this:

  • we want to allow apps sto scale up on demand / dynamically. There shouldn't be a configuration that changes semantics only when the app is a singleton. (and this 2LC isn't clustered)

  • since we can't guarantee consistency at all, with @galderz we decided to simplify the whole cache implementation by removing some complexity which fights specific race conditions. By removing the code which allows strong consistency levels, we actually lose this safety even if the app is run just in a single node mode.

Hence the need to reword some stuff here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we don't want the people having simple applications in the cloud to use Protean?

That sounds very limitative from my POV.


All cached queries are by default kept in a single region dedicated to them called `default-query-results-region`.

All regions are bounded by size and time by default. The defaults are `10000` max entries, and `100` seconds as maximum idle time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we put an idle time by default? I mean the items should be removed from the cache on update, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't added the default, just clarified the text.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsmet But what if the items are not updated, e.g. with read-only entities?


Also, when running multiple copies of the same application (in a cluster, for example on Kubernetes/OpenShift), caches in separate copies of the application aren't synchronized.

For these reasons, enabling caching is only suitable when certain assumptions can be made: we strongly recommend that only entities, collections and queries which never change are cached. Or at most, that when indeed such an entity is mutated and allowed to be read out of date (stale) this has no impact on the expectations of the application.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading that and the sentences below, it seems that you don't consider at all the case where you have only one copy of the application?

This is the case for a lot of applications and IIRC, in this case, the updated entities are updated in the cache and you never get stalled entries? At least this is my recollection that it was the case with the Ehcache implementation.

Maybe having an entirely separate paragraph about consistency and clustering would help to avoid having a negative message when people only have one simple non-clustered application?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above explanation: even single copy is bust (and we shouldn't allow people to assume copies are singletons, that doesn't work safely on Openshift)

@Sanne
Copy link
Member Author

Sanne commented Feb 7, 2019

@galderz @gsmet I need an approval so we can move on, thanks!


All cached queries are by default kept in a single region dedicated to them called `default-query-results-region`.

All regions are bounded by size and time by default. The defaults are `10000` max entries, and `100` seconds as maximum idle time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsmet But what if the items are not updated, e.g. with read-only entities?

@galderz
Copy link
Member

galderz commented Feb 7, 2019

@Sanne Approved

@Sanne Sanne merged commit 1b0f547 into quarkusio:master Feb 7, 2019
@Sanne Sanne deleted the Polish2LCDocs branch February 7, 2019 17:23
@gsmet gsmet added this to the 0.9.0 milestone Feb 11, 2019
maxandersen pushed a commit to maxandersen/quarkus that referenced this pull request Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants