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 hasNoSlots() method to RedisClusterNode #3015

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

wmxl
Copy link
Contributor

@wmxl wmxl commented Oct 15, 2024

Closes #3014

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.

Pull Request: Add hasNoSlots() Method to RedisClusterNode

Feature Request:

This pull request addresses feature request #3014 and is related to the bug report #2769.

Problem:

Currently, checking if a RedisClusterNode has no assigned slots involves accessing the private slots field or using getSlots(), which generates unnecessary objects and adds overhead.

Solution:

This PR introduces a new method, hasNoSlots(), in the RedisClusterNode class. This method encapsulates the logic for checking if the slots field is either null or empty, improving performance and code readability.

Changes:

•	Added the method public boolean hasNoSlots() to the RedisClusterNode class.
•	Added unit tests to validate the behavior of this method.

Testing:

•	All existing tests pass.
•	New unit tests were added to ensure correct behavior when:
•	Slots are null.
•	Slots are empty.
•	Slots are assigned.

@tishun tishun added the type: feature A new feature label Oct 16, 2024
@tishun tishun added this to the 6.5.0.RELEASE milestone Oct 16, 2024
Copy link
Collaborator

@tishun tishun left a comment

Choose a reason for hiding this comment

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

Seems like a good improvement, I have no comments.

BTW if you'd like you can add yourself to the list of authors at line 48

@tishun
Copy link
Collaborator

tishun commented Oct 16, 2024

Oh, yes, you also need to ensure your code is properly formatted:

mvn formatter:format

@wmxl wmxl force-pushed the feature/add-hasNoSlots-method branch from e0db1d0 to 4486c30 Compare October 17, 2024 08:51
@wmxl
Copy link
Contributor Author

wmxl commented Oct 17, 2024

Oh, yes, you also need to ensure your code is properly formatted:

mvn formatter:format

Sorry about that! I’ve fixed the formatting and pushed the changes.

@tishun tishun merged commit f467212 into redis:main Oct 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hasNoSlots() Method to RedisClusterNode
2 participants