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

Add ability to disable local caching #79

Closed
GiedriusS opened this issue Aug 17, 2022 · 5 comments
Closed

Add ability to disable local caching #79

GiedriusS opened this issue Aug 17, 2022 · 5 comments

Comments

@GiedriusS
Copy link

Currently, "cache size each conn" is set to the default value if 0 is provided: https://github.com/rueian/rueidis/blob/master/rueidis.go#L201-L203, https://github.com/rueian/rueidis/blob/8b21fc0a8614397bbe4beada7292f2b9431a9d18/pipe.go#L63. Perhaps we could skip the LRU entirely if zero has been passed? However, that would be a breaking change. Perhaps a separate flag could exist such as DisableLocalCache?

Ran into this while doing thanos-io/thanos#5593. I think some users might not want to cache items locally if their systems don't have much RAM, for example.

@rueian
Copy link
Collaborator

rueian commented Aug 17, 2022

Sure, a separate flag to disable the cache is much easier to implement.

@rueian
Copy link
Collaborator

rueian commented Aug 18, 2022

Hi @GiedriusS,

v0.0.71 now has a ClientOption.DisableCache that will fall back Client.DoCache/Client.DoMultiCache to Client.Do/Client.DoMulti and effectively disables the client-side caching.

In terms of the second thing, There's no way to disable the default TTL in case PTTL fails you mentioned in the thanos-io/thanos#5593. Since the PTTL is sent inside the same MULTI EXEC block of the target command, it is guaranteed that PTTL succeeds if you get the target response.

@GiedriusS
Copy link
Author

I see. I am reading the code a bit now. So, it doesn't matter what time.Duration we pass to MGetCache? 🤔

@rueian
Copy link
Collaborator

rueian commented Aug 19, 2022

Hi @GiedriusS,

The time.Duration you passed will take effect in two cases:

  1. The time.Duration is smaller than PTTL response.
  2. The key on redis has no TTL

@GiedriusS
Copy link
Author

I see, it makes sense! Thank you for the response. I'll continue on with thanos-io/thanos#5593.

appleboy pushed a commit to appleboy/rueidis that referenced this issue Oct 15, 2023
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

No branches or pull requests

2 participants