TopologyComparators performance issue

[alessandrosimi-sa]

Bug Report

Current Behavior

The essentiallyEqualsTo method of the TopologyComparators has a performance issue due to the comparison between the slots. With an aggressive cluster topology refresh interval (500ms) and a single node cluster the machine spends the majority of the CPU on the the essentiallyEqualsTo method and in specific on the o1.getSlots().containsAll(o2.getSlots()) code.

Input Code

I created a this simple class to reproduce the error. In my machine this code takes around 11 seconds.

package io.lettuce.core.cluster.topology;

import io.lettuce.core.cluster.SlotHash;
import io.lettuce.core.cluster.models.partitions.RedisClusterNode;

import java.util.ArrayList;
import java.util.List;

public class SlowTest {

    public static void main(String[] args) {
        long start = System.currentTimeMillis();
        RedisClusterNode nodeA = createNode();
        RedisClusterNode nodeB = createNode();
        for (int i = 0; i < 100; i++) nodeA.getSlots().containsAll(nodeB.getSlots());
        System.out.println(System.currentTimeMillis() - start);
    }

    private static RedisClusterNode createNode() {
        RedisClusterNode nodeA = new RedisClusterNode();
        List<Integer> slots = new ArrayList<>();
        for (int i = 0; i < SlotHash.SLOT_COUNT; i++) {
            slots.add(i);
        }
        nodeA.setSlots(slots);
        return nodeA;
    }
    
}

Expected behavior/code

The comparison should be faster so the cluster topology can be refreshed without impacting the performance of the application.

Environment

• Lettuce version(s): latest (tested on 5.2.0.BUILD-SNAPSHOT)
• Redis version: -

Possible Solution

The equals method of the BitSet should be equivalent so it can be used as alternative.

public boolean hasSameSlotsOf(RedisClusterNode node) {
    if (node == null) return false;
    else return this.slots.equals(node.slots);
}

This method can be part of the RedisClusterNode class.

The test with the new method takes 20~ ms on my machine.

package io.lettuce.core.cluster.topology;

import io.lettuce.core.cluster.SlotHash;
import io.lettuce.core.cluster.models.partitions.RedisClusterNode;

import java.util.ArrayList;
import java.util.List;

public class SlowTest {

    public static void main(String[] args) {
        long start = System.currentTimeMillis();
        RedisClusterNode nodeA = createNode();
        RedisClusterNode nodeB = createNode();
        for (int i = 0; i < 100; i++) nodeA.hasSameSlotsOf(nodeB); // <-- here I'm using the new method
        System.out.println(System.currentTimeMillis() - start);
    }

    private static RedisClusterNode createNode() {
        RedisClusterNode nodeA = new RedisClusterNode();
        List<Integer> slots = new ArrayList<>();
        for (int i = 0; i < SlotHash.SLOT_COUNT; i++) {
            slots.add(i);
        }
        nodeA.setSlots(slots);
        return nodeA;
    }

}

[mp911de]

Thanks for reporting the issue. Care to submit a pull request?

[mp911de]

Thanks a lot for your contribution! That's fixed now.