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

Implement client side caching #521

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

Conversation

holykol
Copy link

@holykol holykol commented Oct 2, 2020

This PR implements server-assisted client side caching introduced in Redis 6.0

This is done entirely at a pool level by running a separate goroutine that subscribes to invalidation broadcasts.
Command responses are, again, cached by middleware at a pool level.

This is done via new Cacher struct in redisx package.

TODO:

  • Cache size limit
  • Broadcasting mode
  • Opt-in caching (via Matcher)
  • Keeping track of TTL

closes #513

@holykol
Copy link
Author

holykol commented Oct 2, 2020

Can someone review?

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given not everyone will be using caching and in fact redis 6 for some time, I'm wondering if this should be implemented by a separate sub package?

This would ensure we don't bloat the core pool code with features not everyone will use or want.

What do you think?

@holykol
Copy link
Author

holykol commented Oct 4, 2020

@stevenh Great idea. I'm a little embarrassed I haven't thought of that before I started trying to fit more code into redis.Pool. Give me some time.

@holykol
Copy link
Author

holykol commented Oct 4, 2020

Didn't mean to close this, but fine. Take a look at now @stevenh

@holykol holykol reopened this Oct 4, 2020
* panic with actual predefined error
* Use LRU cache with TTL support
* Put key's TTL in local cache
* Use pipelining to save RTT
* Improve tests running time
* And some other minor improvements
@holykol
Copy link
Author

holykol commented Oct 13, 2020

OK, I've gave up on trying to come up with abstraction for caching other data types. It's too much work for potentially useless feature:

  1. Implementing this feature basically means re-implementing big chunk of Redis functionality locally.
  2. Hashes, lists and sets change too fast to benefit from caching.
  3. My guess is that most people using Cacher will just store json/protobuf/gob/bare serialized data structures and will not care about other types.

@stevenh Can this be merged with it's current feature set?

@holykol holykol requested a review from stevenh October 13, 2020 21:19
@stevenh
Copy link
Collaborator

stevenh commented Oct 19, 2020

Just a quick note to say I haven't had chance to look at this yet, hopefully soon

redisx/cacher.go Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

About 6.0 feature: client side caching
3 participants