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

docs: add default threads count about NioEventLoopGroup #3221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brido4125
Copy link

@brido4125 brido4125 commented Mar 19, 2025

add NioEventLoopGroup default threads counts in AbstractRedisClient javadoc.

Make sure that:

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

@tishun
Copy link
Collaborator

tishun commented Mar 24, 2025

Hey @brido4125 ,
can we take a step back and discuss the issue.

I assume you found it hard to understand the defaults, correct? Would it make sense to add this to the documentation instead (not sure how many users dig in the source code to that level) ?

@tishun tishun added the status: waiting-for-feedback We need additional information before we can continue label Mar 24, 2025
@brido4125
Copy link
Author

I assume you found it hard to understand the defaults, correct?

Yes, exactly what i intended.
I wanted to find default value for NioEventLoopGroup
because the default value for EpollEventLoopGroup is mentioned with same paragraph.

Would it make sense to add this to the documentation instead (not sure how many users dig in the source code to that level) ?

I agree.
where can i find the docs about RedisCilent or NioEventLoopGroup config?
(I cannot find proper documentation in lettuce/docs dir)

@tishun
Copy link
Collaborator

tishun commented Mar 25, 2025

Yes, exactly what i intended. I wanted to find default value for NioEventLoopGroup because the default value for EpollEventLoopGroup is mentioned with same paragraph.

This comment is actually quite ancient (comes from the year of 2015) and does not seem to be entirely correct, as per the current code base. I would rephrase the entire JavaDoc with something less specific, because the AbstractRedisClient is no longer involved in configuring the event loop threads. Instead the DefaultClientResources has all this logic in it.

The number of threads for both I/O and computation are selected in the following way:

  • any setting in the ClientOptions takes first precedence, otherwise we fallback to defaults
  • if there is an environment variable setting for io.netty.eventLoopThreads we use it as default, otherwise ...
  • ... we take the number of available processors, retrieved by Runtime.getRuntime().availableProcessors()
  • in any case if the chosen number is less than the minimum, which is 2 threads, then we use the minimum

So the setting for both I/O and computation threads are based on the presence and value of the above settings.

I agree. where can i find the docs about RedisCilent or NioEventLoopGroup config? (I cannot find proper documentation in lettuce/docs dir)

I think the best I can find was https://redis.github.io/lettuce/advanced-usage/#configuration-settings
However it does not mention all the details. It also seems to be outdated and states that the minimum is 3 and not 2, which was changed in #1278

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-feedback We need additional information before we can continue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants