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

Redis cache is not supporting with null values #39547

Open
amitgol1 opened this issue Mar 19, 2024 · 11 comments
Open

Redis cache is not supporting with null values #39547

amitgol1 opened this issue Mar 19, 2024 · 11 comments
Labels

Comments

@amitgol1
Copy link

Describe the bug

In-memory cache :
Annotation of @CacheResult on method that return null value will throw exception from redis cache, how to put the redis cache in ignore mode fro null values?

Expected behavior

Ignore if value is null

Actual behavior

Exception about null in the cache

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@amitgol1 amitgol1 added the kind/bug Something isn't working label Mar 19, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 19, 2024

/cc @cescoffier (redis), @gsmet (redis), @gwenneg (cache), @machi1990 (redis)

@cescoffier
Copy link
Member

Redis does not support 'null'. It's not part of the protocol. The semantic can be ambiguous:

  • should it remove the entry
  • should it set it to a sentinel value representing 'null'

This, I don't think we will fix it.

@karesti
Copy link
Member

karesti commented Apr 4, 2024

@cescoffier Spring Cache SPI allows telling if we want to cache or ignore null values. Even if a null value can't be added into the key/value in a cache provider ( which is the case for Redis and Infinispan for example), we can add an special wrapper representing a null value.
Null value caching can be enabled or disabled too on the SPI.
I would consider exploring this in the caching SPI

@karesti
Copy link
Member

karesti commented Apr 5, 2024

@cescoffier @geoand @gwenneg

Concerning the null values the cache extension states: Null value caching is supported

https://quarkus.io/guides/cache#negative-cache

Some approaches to caching with annotations involve conditional caching through metadata. The provided annotations allow for the explicit caching of method returns based on certain conditions, including unless statements and options for caching null values.

In our current cache extension, I believe a complex conditional caching mechanism might be unnecessary, at least for now. However, the issue of caching null returns versus non-null returns is a common problem worth considering.
Including a property property to disable the null value caching in the @CacheResult annotation, maybe ?

@cescoffier
Copy link
Member

I don't believe we should encourage caching null but instead recommend that the user use a custom sentinel value in this case. If null is supported by the caching provider, it can use null as a sentinel value; otherwise, a custom (non-null) sentinel value will need to be used.

@karesti
Copy link
Member

karesti commented Apr 8, 2024

@cescoffier what if you want to cache unless it's null ? This is a common use case

@cescoffier
Copy link
Member

In this case, we would not cache null right? You will only cache the value that is not null.

@karesti
Copy link
Member

karesti commented Apr 8, 2024

As I said in my comment, the caching extension states that allows for storing null values, thus caching providers should do that too. However, there are cases where some methods return null, and caching that result is desirable, while in other cases, we prefer not to cache null to ensure method execution. This feature of conditional caching is commonly needed and enables us to control caching behavior as needed.

@cescoffier
Copy link
Member

I agree; that's the ambiguity behind null. You do not know if you want to cache it or ignore that call completely.

@amitgol1
Copy link
Author

Thanks for the details that you shared. @karesti @cescoffier

What is the conclusion? Can you provide a flag to ignore null if we want?

@cescoffier
Copy link
Member

@amitgol1 No, no conclusion so far.

I'm still not convinced that caching null is correct. Too much ambiguity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants