Skip to content

Conversation

@andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Apr 10, 2025

DOC-5064

I've added four new short sections, partly based on suggestions in Ricardo Ferreira's recent blog post and an experimental "live" checklist. I've also expanded the description of timeouts a bit. All feedback is welcome, as usual, especially regarding the following points:

  • We could also add failover as a recommendation, but it is a common requirement? Might be misleading if we recommend it but most people don't need to use it.
  • I think retries are handled by connection pools but if it's worth explicitly mentioning them then I'll add a specific section.
  • Should TLS connection be a production recommendation, or is it something that people should just add if they need it?

@andy-stark-redis andy-stark-redis requested review from a team and mortensi April 10, 2025 15:02
@andy-stark-redis andy-stark-redis self-assigned this Apr 10, 2025
@github-actions
Copy link
Contributor

Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

Just a couple of items to validate. I like the checklist short code! Approving now so you're not held up.

@andy-stark-redis
Copy link
Contributor Author

Well spotted, @dwdougherty - thanks!

@andy-stark-redis andy-stark-redis requested a review from uglide April 22, 2025 09:01
JedisClientConfig clientConfig = DefaultJedisClientConfig.builder()
.connectionTimeoutMillis(2000)
.socketTimeoutMillis(2000)
.tcpKeepAlive(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

tcpKeepAlive is by default through when using DefaultJedisSocketFactory and is not externally configurable.
DefaultJedisClientConfig does not expose tcpKeepAlive(true) method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ggivo Fixed.

@andy-stark-redis andy-stark-redis requested a review from ggivo April 30, 2025 09:24
Copy link
Contributor

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM

@andy-stark-redis
Copy link
Contributor Author

@ggivo Awesome, thanks! I'll merge this now, but please let me know if you think of any other good production advice and I'll add it to the page.

@andy-stark-redis andy-stark-redis merged commit 75b72bd into main Apr 30, 2025
5 checks passed
@andy-stark-redis andy-stark-redis deleted the DOC-5064-jedis-prod-usage branch April 30, 2025 10:33
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.

4 participants