Skip to content

Feature/csc ttl support #4120

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

PengJingzhao
Copy link

Hello, I add a ttl support for the jedis csc (extend the DefaultCache) to solve a issue of the jedis.You can read my commit record for the details

@PengJingzhao
Copy link
Author

If somebody help me review it? Thankyou very much!

@ggivo
Copy link
Collaborator

ggivo commented Mar 24, 2025

Related to #4117

@ggivo ggivo mentioned this pull request Mar 24, 2025
@ggivo
Copy link
Collaborator

ggivo commented Mar 28, 2025

There is an ongoing discussion regarding adding TTL to CSC in Jedis tracked with #4117
Can we take a step back and try to identify what is the issue we are trying to solve before getting to the implementation?

@PengJingzhao
Copy link
Author

Of course! I can explain in detail what my PR does. In my PR, I extend the DefaultCache class under the csc package. In the extended class, I use a Map called expirationTimes to maintain an expiration time for each cache key. Additionally, I start a thread to periodically clean up keys that have already expired.

@ggivo
Copy link
Collaborator

ggivo commented Mar 28, 2025

My question was about the problem we want to solve before going into concrete implementations.
CSC is not intended to be a generic cache and is driven by notification from Redis server. Instead of providing an TTL implementation inside Jedis, another approach is to use integrate a third-party cache library instead of the DefaultCache implementstion provided.

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

Successfully merging this pull request may close these issues.

2 participants