Infinispan - Support caching annotations

[karesti]

Caching annotations @CacheInvalidate @CacheInvalidateAll and @CacheResult

https://issues.redhat.com/browse/ISPN-13062

[gsmet]

@karesti thanks but could you split that in two different commits? If we have to backport an Infinispan upgrade, I would like to be able to do it easily. Thanks.

[karesti]

Infinispan Version Update https://github.com/quarkusio/quarkus/pull/25306

[karesti]

@gwenneg I made locally the small review changes and I will check the 2 PR you mentioned to report the changes here too. I ping you once is ready

[geoand]

Hey,

What's the status of this? This would be a super interesting feature and something that has been requested from the community.

[karesti]

@geoand I've been sidetracked too much recently, I need to report the changes reported by @gwenneg

[geoand]

Gotcha, thanks for the update

[karesti]

@geoand give me some days to get back to it properly. Sorry again

[geoand]

NP!

[geoand]

Closed by accident, sorry :)

[karesti]

@geoand @gwenneg FYI coming back to work on this fully for the next days (and other features preparing the integration with Infinispan 14)

[geoand]

👍

[quarkus-bot[bot]]

---

:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.

---

Failing Jobs - Building 5a3e1a32426d329117cfdfb58fba21c954d36bd2

Status	Name	Step	Failures	Logs	Raw logs
✖	Initial JDK 11 Build	Build	Failures	Logs	Raw logs

Failures

:gear: Initial JDK 11 Build #

- Failing: extensions/infinispan-client/runtime 
! Skipped: devtools/bom-descriptor-json docs extensions/infinispan-client/deployment and 4 more

:package: extensions/infinispan-client/runtime

✖ Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.20.0:validate (default) on project quarkus-infinispan-client: File '/home/runner/work/quarkus/quarkus/extensions/infinispan-client/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/CacheResultInterceptor.java' has not been previously formatted. Please format file (for example by invoking `mvn -f extensions/infinispan-client/runtime net.revelc.code.formatter:formatter-maven-plugin:2.20.0:format`) and commit before running validation!

[quarkus-bot[bot]]

---

:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.

---

Failing Jobs - Building 7b75cb308dcdca020f2cf3f02ce69024121485e8

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	Build	Failures	Logs	Raw logs
✖	JVM Tests - JDK 17	Build	Failures	Logs	Raw logs
✖	JVM Tests - JDK 18	Build	Failures	Logs	Raw logs
✖	Native Tests - Cache	Build	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

:gear: JVM Tests - JDK 11 #

- Failing: integration-tests/infinispan-client 

:package: integration-tests/infinispan-client

✖ io.quarkus.it.infinispan.client.InfinispanClientFunctionalityTest.testCacheAnnotations line 60 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

---

:gear: JVM Tests - JDK 17 #

- Failing: integration-tests/infinispan-client 

:package: integration-tests/infinispan-client

✖ io.quarkus.it.infinispan.client.InfinispanClientFunctionalityTest.testCacheAnnotations line 60 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

---

:gear: JVM Tests - JDK 18 #

- Failing: integration-tests/infinispan-client 

:package: integration-tests/infinispan-client

✖ io.quarkus.it.infinispan.client.InfinispanClientFunctionalityTest.testCacheAnnotations line 60 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

---

:gear: Native Tests - Cache #

- Failing: integration-tests/infinispan-client 

:package: integration-tests/infinispan-client

✖ io.quarkus.it.infinispan.client.InfinispanClientFunctionalityIT.testCacheAnnotations - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

[karesti]

@geoand I need to check the failing test :) I did not run them properly as I thought yesterday, sorry about it

[geoand]

No problem :)

[geoand]

CI is passing. Is this ready for a review?

[geoand]

One thing I would like us to have in mind for a follow-up, is to provider proper metrics (as we do for Caffeine)

[karesti]

@geoand yes please

[karesti]

One thing I would like us to have in mind for a follow-up, is to provider proper metrics (as we do for Caffeine)

yes, we can totally create an issue to make it happen after that

[geoand]

Dump question that has probably been answered before: Do we really want to have a separate set of Infinispan specific annotations instead of reusing the Quarkus Cache annotations?

[gwenneg]

Assuming that's possible (I didn't check), Infinispan could even be a caching provider from quarkus-cache that could be enabled from the quarkus-cache config when the Infinispan extension is on the classpath. Quarkus-cache was built with that kind of integration case in mind.

Unfortunately, this PR already shows that maintaining the duplicated code (annotations and interceptors) won't be an easy task.

[karesti]

@gwenneg there are limitations concerning the local cache of caffeine and how it works with infinispan. For now, I think we need to implement this feature appart from quarkus-cache and then try to implement caching providers of course.

I will report the changes @geoand and @gwenneg hopefully this will be enough to integrate the feature in the infinispan extension

[geoand]

Then maybe we need a set of annotation for remote caching specifically?

[karesti]

@geoand we might want to think about this but for now the Infinispan team would really like to offer this first implementation in the extension and improve the use through users as well. This feature is taking way too long to overthink it too much right now IMO

[geoand]

Yeah, I understand, but would it not make sense to provide the remote caching annotations now? It should not be too much work (theoretically) and it would shield us from moving / breaking the annotations in the future.

I won't block the PR on this suggestion, but I highly suggest it should be given serious thought (cc @gsmet)

[karesti]

@geoand I agree on working together towards that goal. I'm reporting the missing changes now, I ping you once it's done! cc @gwenneg

[geoand]

Thanks!

[karesti]

@geoand @gwenneg how does it look now ?

[geoand]

I'll be on PTO for the next 10 days or so

[gwenneg]

I just came back from PTO. I'll have a look at this PR later today.