quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.54k stars 2.61k forks source link

Quarkus Cache (Caffeine): Allow setting refreshAfterWrite property #25921

Open davefranken-ah opened 2 years ago

davefranken-ah commented 2 years ago

Description

Currently, the Quarkus caffeine cache offers the following properties (see https://quarkus.io/guides/cache#quarkus-cache-config-group-cache-config-caffeine-config_configuration):

Caffeine also offers setting 'refresh after write' (see https://github.com/ben-manes/caffeine/wiki/Refresh).

Implementation ideas

I did some research and the version of Caffeine (2.9.3) does support this already in the builder which is called from io.quarkus.cache.runtime.caffeine.CaffeineCacheImpl so it should be fairly trivial to add this property. We need to add it to CaffeineCacheInfo (I think).

quarkus-bot[bot] commented 2 years ago

/cc @gwenneg

geoand commented 2 years ago

@davefranken-ah would you be willing to contribute this?

davefranken-ah commented 2 years ago

@davefranken-ah would you be willing to contribute this?

Yeah no problem, I can try to make a PR, but might need some help, I'll see if I can get it to work.

gwenneg commented 2 years ago

Thanks @davefranken-ah! Don't hesitate to ask for help :smiley:

Davio commented 2 years ago

@gwenneg It seems there is something weird going on with the build, when I try to run the build with ./mvnw -f extensions/cache clean install I get an error:

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   DeploymentExceptionsTest.lambda$static$1:44 expected: <javax.enterprise.inject.spi.DeploymentException> but was: <java.lang.IllegalArgumentException>
[ERROR]   UnknownCacheTypeTest Build failed with a wrong exception, expected class javax.enterprise.inject.spi.DeploymentException but got java.lang.IllegalArgumentException: Undeclared build item class io.quarkus.deployment.builditem.TransformedClassesBuildItem ==> expected: <true> but was: <false>
[ERROR] Errors: 
[ERROR]   CacheConfigTest » Runtime java.lang.IllegalArgumentException: Undeclared build...
[ERROR]   DisabledCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared bui...
[ERROR]   CacheHotReloadTest.testHotReload » Runtime java.lang.RuntimeException: java.la...
[ERROR]   CacheKeyGeneratorTest » Runtime java.lang.IllegalArgumentException: Undeclared...
[ERROR]   CacheNamesTest » Runtime java.lang.IllegalArgumentException: Undeclared build ...
[ERROR]   CompletionStageReturnTypeTest » Runtime java.lang.IllegalArgumentException: Un...
[ERROR]   ConcurrencyTest » Runtime java.lang.IllegalArgumentException: Undeclared build...
[ERROR]   DefaultKeyCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared b...
[ERROR]   ExplicitCompositeKeyCacheTest » Runtime java.lang.IllegalArgumentException: Un...
[ERROR]   ExplicitSimpleKeyCacheTest » Runtime java.lang.IllegalArgumentException: Undec...
[ERROR]   ImplicitCompositeKeyCacheTest » Runtime java.lang.IllegalArgumentException: Un...
[ERROR]   ImplicitSimpleKeyCacheTest » Runtime java.lang.IllegalArgumentException: Undec...
[ERROR]   MultiValueTest » Runtime java.lang.IllegalArgumentException: Undeclared build ...
[ERROR]   MultipleCacheAnnotationsTest » Runtime java.lang.IllegalArgumentException: Und...
[ERROR]   NoOpCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared build i...
[ERROR]   NullKeyOrValueCacheTest » Runtime java.lang.IllegalArgumentException: Undeclar...
[ERROR]   ProgrammaticApiTest » Runtime java.lang.IllegalArgumentException: Undeclared b...
[ERROR]   SharedCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared build...
[ERROR]   SpecializedCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared ...
[ERROR]   ThrowExecutionExceptionCauseTest » Runtime java.lang.IllegalArgumentException:...
[ERROR]   UniReturnTypeTest » Runtime java.lang.IllegalArgumentException: Undeclared bui...
[ERROR]   ZeroCaffeineCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared...
[ERROR]   ZeroNoOpCacheTest » Runtime java.lang.IllegalArgumentException: Undeclared bui...
[INFO] 
[ERROR] Tests run: 34, Failures: 2, Errors: 23, Skipped: 0

This error also occurs on the main branch so it seems unrelated to my changes.

gwenneg commented 2 years ago

Did you rebuild Quarkus entirely? The build is fine on my machine and on CI so I suppose something's wrong on your machine.

gilvansfilho commented 2 years ago

Here build works fine too.

I made some code to address this issue but now I am looking for more information about CacheLoader. Someone could help? Perhaps I could submit PR in WIP mode and we discuss there?

gastaldi commented 2 years ago

I made some code to address this issue but now I am looking for more information about CacheLoader. Someone could help? Perhaps I could submit PR in WIP mode and we discuss there?

Sounds good

davefranken-ah commented 2 years ago

I can run the build with -Dquickly just fine, it's just that the tests keep failing, this is with both Eclipse Temurin JDK 11 and Mandrel 22.11 on Linux 64-bit. And it's not only the cache tests, when I run the entire build with ,/mvnw install mutiny fails to build as well. I studied the contributing.md file, but nothing in there can help me sadly.

geoand commented 2 years ago

@davefranken-ah were you able to build the project?

technical-debt-collector commented 1 year ago

This might come off as naive but what if Quarkus just exposed the builder somehow? Developers requiring any Caffeine functionality currently not made available by Quarkus could just configure this themselves using the builder and then leave that to Quarkus?

My team is also interested in this feature, as well as being able to intercept evictions as documented here: https://github.com/ben-manes/caffeine/wiki/Compute

geoand commented 1 year ago

I think that's a reasonable request (and something we allow in other extensions as well).

@gwenneg WDYT?

gwenneg commented 1 year ago

That sounds reasonable indeed.

geoand commented 1 year ago

I'll see if I can come up with something for Quarkus 3.0, but I can't make any promises as there is a lot of work to be done for that release.

technical-debt-collector commented 1 year ago

If it's of any help I could try to have a look at it myself, although I too would probably need some pointers along the way. Unless @davefranken-ah wants to give this another shot?

geoand commented 1 year ago

If I can't get to this by early next week, I'll let you know :)

gsmet commented 1 year ago

Even if we expose the builder, I think it would make sense to have that property.

geoand commented 1 year ago

I'll do the builder, someone else can do the property 😉

geoand commented 1 year ago

I opened https://github.com/quarkusio/quarkus/pull/31754 as a draft because there is still an issue to be resolveds (see description)

geoand commented 1 year ago

So in the meantime, I'll go ahead and do another PR adding the property

geoand commented 1 year ago

Actually, adding the property does not work either because we get:

Caused by: java.lang.IllegalStateException: refreshAfterWrite requires a LoadingCache
        at com.github.benmanes.caffeine.cache.Caffeine.requireState(Caffeine.java:203)
        at com.github.benmanes.caffeine.cache.Caffeine.requireNonLoadingCache(Caffeine.java:1160)
        at com.github.benmanes.caffeine.cache.Caffeine.buildAsync(Caffeine.java:1091)
        at io.quarkus.cache.runtime.caffeine.CaffeineCacheImpl.<init>(CaffeineCacheImpl.java:77)
        at io.quarkus.cache.runtime.caffeine.CaffeineCacheManagerBuilder$1.get(CaffeineCacheManagerBuilder.java:57)
gwenneg commented 1 year ago

We discussed the property in https://github.com/quarkusio/quarkus/pull/25956. I'm not in favor of adding that because of the reason explained in that PR.

geoand commented 1 year ago

👌

gastaldi commented 1 year ago

Perhaps we should close this issue then?

ayushman2001 commented 11 months ago

is there any development going on this feature

chir050 commented 7 months ago

It would be nice to know when it is planned to be implemented?