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

DOC-4037 Rework client-side caching content #530

Merged
merged 17 commits into from
Sep 27, 2024

Conversation

andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Aug 8, 2024

DOC-4037

This isn't complete yet. In particular, I wanted to see what we all think about the Python page content before adding the equivalent stuff for the other clients. Also, a few other questions:

  • Is the sequence diagram on the CSC intro page overkill? I find them quite intuitive to read, but maybe not everyone else likes them.
  • Is the list of data that gets cached correct and do we want any more/fewer examples here?
  • Do we want to document more advanced options like supplying your own cache implementation and eviction policies, etc? If so then we might move the usage recommendations out of the intro and put them in another page with the advanced options.
  • I've moved the existing technical CSC page to Develop/Reference but I haven't changed it much. Does this page need some attention too or is it OK for what it is?

All feeback welcome :-)

@dwdougherty
Copy link
Collaborator

Before I read the rest of the material, I did notice that the image files are going straight into /static/images. I suggest you create a directory in images to house these image files. I don't know what to call it, but I'm open to suggestions; maybe "dev" or "ce" (for community edition).

On a somewhat related note, I need to work on moving other stray images to /static/images/[dev|ce|whatever].

@andy-stark-redis
Copy link
Contributor Author

@dwdougherty I've put them in a csc folder for now but as you say, maybe we need to structure the images folder better.


Accessing the cache is much faster than communicating with the database over the
network. Also, this technique reduces the load on the database server, so you may
be able to run it using fewer nodes.
Copy link
Member

Choose a reason for hiding this comment

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

Using fewer resources? In Enterprise and CE there are different approaches to address scalability (more shards, more nodes). Also, we can mention that CSC achieves lowering bandwidth consumption and expenses

These types are designed to be updated frequently, which means that caching
them gives little or no benefit.
- Non-deterministic commands such as [`HSCAN`]({{< relref "/commands/hscan" >}})
and [`ZRANDMEMBER`]({{< relref "/commands/zrandmember" >}}). By design, these commands
Copy link
Member

Choose a reason for hiding this comment

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

I think we can mention HGETALL, being a popular one. @uglide can you validate this?

object they refer to.


## Enabling CSC
Copy link
Member

Choose a reason for hiding this comment

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

This is not required because the client library enables the tracking at the connection establishment time. So it is necessary to just configure the connection

messages can be greater than the savings made by caching. Avoid this problem
by using a separate connection *without* CSC for any data that is
not cache-friendly.
- **Set a maximum time-to-live (TTL) for cached items**: The client libraries
Copy link
Member

Choose a reason for hiding this comment

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

The TTL is not supported in this phase, so it has been removed recently. cc @uglide

Copy link
Member

Choose a reason for hiding this comment

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

Overall, this is a great and simple addition. The example will be validated when the clients are stable (TTL has been removed). Per the client library example, we may propose the following examples.

  1. Connection instantiation and configuration
  2. Pool instantiation and configuration
  3. Three example with string, hash, and JSON
  4. Cache flush
  5. Remove one entry from the cache

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortensi For 1) and 2): Are the pool and cluster connections any different, aside from protocol: 3 and the cache_xxx parameters? I've mentioned that you can use the same parameters with all of the different connection variants. I'd prefer not to have all combinations of all the connection options, but if you think there is something significantly different about this then I'll certainly add it.

For 3): There's already an example for string, just to give a simple demonstration of how you can see the cache working using MONITOR. I'm not sure it would help to have hash and JSON examples here, given that they are the same as the usual commands from a coding point of view (again, if I'm wrong about this then please let me know!) There is already a link to the CSC intro, where I discuss the way different datatypes/commands affect the cache (eg, hash fields cached separately from the whole object).

For 4) and 5): Yes, definitely - if there are other cache commands then we should document them. I was also thinking that the options for supplying your own cache implementation and selecting an eviction policy should be documented somewhere. Maybe in a "advanced" page or an advanced section in the intro?

Copy link
Member

Choose a reason for hiding this comment

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

Do you envision a quick start with the tabbed examples in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortensi I wanted to check exactly what you want in the tabbed samples. Are you thinking of stuff from the Google doc like:

client.set("hola", "mundo");
client.set("hello", "world");
client.mget("hola", "hello"); // read from the server
client.mget("hola", "hello"); // cache hit
client.mget("hello", "hola"); // read from server, the order matters
.
.

...or are there other use cases you want to have samples for? I know the Google doc has quite a few examples for Python and Jedis but I'm not sure if we need them in the doc page. Maybe we could have a concrete example at the bottom of some of the entries in this list for extra clarity? I'm not sure that having tabbed samples for these in each language is necessary either, because the samples don't actually do anything useful in themselves, so people wouldn't normally copy/paste them.

However, if we do decide that tabbed samples are a good thing here then I'm confident we can get them ready in good time for the release.

Copy link

@yoavredis yoavredis left a comment

Choose a reason for hiding this comment

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

@andy-stark-redis LGTM.
I approve any changes you will make following the comments by @mortensi

Copy link
Contributor

@kaitlynmichael kaitlynmichael left a comment

Choose a reason for hiding this comment

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

Just some grammar and style suggestions. Besides those, everything is explained well and easy to understand.

content/develop/connect/clients/client-side-caching.md Outdated Show resolved Hide resolved
content/develop/connect/clients/client-side-caching.md Outdated Show resolved Hide resolved
content/develop/connect/clients/client-side-caching.md Outdated Show resolved Hide resolved
content/develop/connect/clients/client-side-caching.md Outdated Show resolved Hide resolved
content/develop/connect/clients/client-side-caching.md Outdated Show resolved Hide resolved
content/develop/connect/clients/java/jedis.md Outdated Show resolved Hide resolved
content/develop/connect/clients/java/jedis.md Show resolved Hide resolved
content/develop/connect/clients/java/jedis.md Show resolved Hide resolved
content/develop/connect/clients/python/redis-py.md Outdated Show resolved Hide resolved
content/develop/connect/clients/python/redis-py.md Outdated Show resolved Hide resolved
andy-stark-redis and others added 6 commits September 23, 2024 09:31
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
@andy-stark-redis andy-stark-redis merged commit 50c39d8 into main Sep 27, 2024
4 checks passed
@andy-stark-redis andy-stark-redis deleted the DOC-4037-rework-csc-content branch September 27, 2024 16:44
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.

5 participants