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 #3014

Closed
wmxl opened this issue Oct 15, 2024 · 1 comment · Fixed by #3015
Closed

Add hasNoSlots() Method to RedisClusterNode #3014

wmxl opened this issue Oct 15, 2024 · 1 comment · Fixed by #3015
Labels
type: feature A new feature
Milestone

Comments

@wmxl
Copy link
Contributor

wmxl commented Oct 15, 2024

Feature Request

Is your feature request related to a problem? Please describe

In bug report #2769, an issue was identified when handling Redis Cluster topology refreshes during node reconfigurations where nodes temporarily had no assigned slots. The maintainer suggested subclassing RedisClusterClient and overriding determinePartitions to address the issue. I followed the suggestion and initially used redisClusterNode.getSlots().isEmpty() to identify this case. However, I found that getSlots() is quite time-consuming, increasing the overhead of topology refreshes.

Describe the solution you'd like

To improve efficiency, I propose adding a method hasNoSlots() toRedisClusterNode. This method has much better performance than getSlots().isEmpty()

Proposed Method:
/**
     * Checks if the node has no slots assigned.
     *
     * @return {@code true} if the slots field is null or empty, {@code false} otherwise.
     */
    public boolean hasNoSlots() {
        return slots == null || slots.isEmpty();
    }

Describe alternatives you've considered

Currently, the alternative is to use getSlots().isEmpty() to determine if a node has no slots assigned. However, this approach is inefficient because it involves generating a new list object, which adds overhead during topology refreshes.

Teachability, Documentation, Adoption, Migration Strategy

This feature request aims to improve the efficiency and readability of RedisClusterNode's API, particularly in scenarios involving cluster scaling and topology adjustments. I appreciate your consideration and am happy to make any additional changes or provide further details if needed.

@wmxl wmxl changed the title Title: Add hasNoSlots() Method to RedisClusterNode Add hasNoSlots() Method to RedisClusterNode Oct 16, 2024
@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
@tishun
Copy link
Collaborator

tishun commented Oct 16, 2024

I think this change makes sense.

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 a pull request may close this issue.

2 participants